Repository navigation
add support for reading credentials from environment variables in @MavenRepository - #240
Closed
leonschreuder wants to merge 2 commits into
Closed
leonschreuder wants to merge 2 commits into
leonschreuder wants to merge 2 commits into
Conversation
leonschreuder
force-pushed
the
env_var_creds
branch
from
September 16, 2019 11:38
68de97f to
d593d97
Compare
forrestbice
added a commit
to forrestbice/kscript
that referenced
this pull request
Oct 12, 2019
Following from: kscripting#240 commits/d593d97c0db19b0f18aef6dc7fb7bc030b63384f.
holgerbrandl
pushed a commit
that referenced
this pull request
Oct 20, 2019
* Create a default Run Configuration for Idea Create a default Run Configuration for Intellij when using the `idea` option. Achieved by creating a `Main` configuration within `.idea/runConfigurations`. The `userArgs` are also passed along by default. * Fix Incorrect Path In Script Tests Following from: #240 commits/d593d97c0db19b0f18aef6dc7fb7bc030b63384f.
Collaborator
|
Nice enhancement. Would you be willing to also add a small note to the README about this new feature? If so, I'd wait with the merge. |
Collaborator
|
Duplicate of #248. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #214 support for credentials in @mavenrepository was added. This PR extends that functionality to optionally provide an environment variable in stead of the value directly.
The implementation uses the
user="{{VAR}}"syntax. It would have been nicer to support literal script calls, likeuser="${System.getEnv("VAR")}", but this would require writing a custom templating engine (see https://stackoverflow.com/a/52183931/3968618) and compiling the code, but that is a lot of work for variable replacement and I also felt it would be risky to allow script injection here. On the other hand, using the basic form of kotlin variable syntax (e.g. "$ENV_VAR") but not any of the others (e.g. with logic in "${}") would be confusing for users.Therefore I decided that a custom syntax would be the best option here, but I am open to feedback.