-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable dcf files to contain R code that can be evaluated upon read #169
Conversation
Thank you @connectedblue for publishing your solution and setting up this PR. I'm including the code below, which is the solution I would have developed, in case it may be relevant in some way. It is an example of an alternative approach that doesn't require backticks.
As an aside, I have no idea how safe it is to run the eval method unchecked. |
@kezkankrayon it was a nice, simple solution you proposed to hide the usernames and passwords in R code, so I think it's a great addition to I added some generic test cases to the regression tests, but not The reason I chose the back ticks was to be consistent with what most R users would recognise from RMarkdown - that stuff inside gets evaluated. It's also somewhat of an advanced feature, so users who need it need to think carefully about it. I agree that running eval uncheckedcan be unsafe (but Your code would work, but I'm not sure what the side effect would be if the config parameter also happened to be valid R - e.g. the string Look forward to hearing your report back on the sql use case. |
@connectedblue, I installed the template via devtools, but encountered the following error when executing
I have not yet been able to determine if this error is a product of the update or if there is an issue else where. So far I've done the following tests:
However, the error remains the same. In short, I'm not sure what has changed and why plain text variables no longer work. |
@kezkankrayon thanks for testing. It's always worth looking at specific use cases. Can you paste the |
@connectedblue, please find the
This .sql file worked on the 26th when user and password were plain strings. But, hasn't worked since installing the template via devtools and reinstalling via I can confirm the ODBC works and setup is correct with the following.
|
Hmmm, there's nothing obvious that I can see, although I don't use odbc myself, so I could be missing something. I'm not sure what the difference is between There has been a recent change to this file, but that was in the SQLite section, not odbc. I wonder if @crubb has some expertise to offer to help diagnose the problem? If it's happening on the CRAN version also, then it can't be due to this recent change, but I'd like to get to the bottom of it. |
Hey guys, the changes were only in the sqlite Best regards, |
Hey, I was able to retry this on another computer today with the same |
Thanks @kezkankrayon for testing this out - good to know it solves the problem of stored passwords in the odbc case. I'm sure it will be helpful in other cases too. |
This PR proposes a change referenced in #30 and recently commented upon by @kezkankrayon.
A generic change is made to the
translate.dcf
function which is used in a number of places to read DCF files. In particular, this is used by the SQL reader where it was requested to support user names and passwords in environment variables.The change is that any item in a DCF file contained in back ticks is evaluated as R code and the result used in place for that variable. This is a similar pattern to that used in R Markdown documents for evaluating code.
As an example, if a DCF file looks like this:
then the
user
andpasswd
variables are set with the results of the environment variables.A test case was also written to test the functionality, and the man page documentation is updated.