-
Notifications
You must be signed in to change notification settings - Fork 5
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
[GH-29] Read credentials from ENV or secrets. #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a few suggestions with some Groovy syntax sugar to simplify your code, you can take them or leave them. Assuming you tested it and it works, looks good to me 👍
plugins/nf-float/src/main/com/memverge/nextflow/FloatConf.groovy
Outdated
Show resolved
Hide resolved
plugins/nf-float/src/main/com/memverge/nextflow/FloatConf.groovy
Outdated
Show resolved
Hide resolved
addresses = (node.address as Collection).collect { | ||
it.toString() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addresses = (node.address as Collection).collect { | |
it.toString() | |
} | |
addresses = node.address*.toString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you check the type with instanceof
like this, you don't have to cast it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use your change but looks like it cannot pass the compile.
[Static type checking] - Spread operator can only be used on collection types
@ line 77, column 29.
addresses = node.address*.toString()
I updated it to something like this:
if (node.address instanceof Collection) {
addresses = node.address.collect {it.toString()}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, I guess the compiler wasn't as smart as I hoped
plugins/nf-float/src/main/com/memverge/nextflow/FloatConf.groovy
Outdated
Show resolved
Hide resolved
Enable the plugin to read credentials from the environment variable: * MMC_ADDRESS: address of the op center * MMC_USERNAME: login username * MMC_PASSWORD: login password Hide the password in the log. Add samples in the README to illustrate how to input the credentials with NextFlow secrets.
I added a test to verify the env changes. |
Looks great! |
Release 0.1.8 includes multiple bugfixes: * [GH-20] Update project's org. name. by @jealous in MemVerge/nf-float#21 * [nextflow-ioGH-22] Include auto-install in the document by @jealous in MemVerge/nf-float#23 * [nextflow-ioGH-26] Support config in process. by @jealous in MemVerge/nf-float#33 * [nextflow-ioGH-32] Upload bin directory. by @jealous in MemVerge/nf-float#34 * [nextflow-ioGH-28] Correct syntax in README. by @jealous in MemVerge/nf-float#36 * [nextflow-ioGH-29] Read credentials from ENV or secrets. by @jealous in MemVerge/nf-float#35 * [nextflow-ioGH-38] Track job with tags by @jealous in MemVerge/nf-float#39 * [nextflow-ioGH-37] Support default registry. by @jealous in MemVerge/nf-float#40 **Full Changelog**: MemVerge/nf-float@0.1.7...0.1.8
Release 0.1.8 includes multiple bugfixes: * [GH-20] Update project's org. name. by @jealous in MemVerge/nf-float#21 * [GH-22] Include auto-install in the document by @jealous in MemVerge/nf-float#23 * [GH-26] Support config in process. by @jealous in MemVerge/nf-float#33 * [GH-32] Upload bin directory. by @jealous in MemVerge/nf-float#34 * [GH-28] Correct syntax in README. by @jealous in MemVerge/nf-float#36 * [GH-29] Read credentials from ENV or secrets. by @jealous in MemVerge/nf-float#35 * [GH-38] Track job with tags by @jealous in MemVerge/nf-float#39 * [GH-37] Support default registry. by @jealous in MemVerge/nf-float#40 **Full Changelog**: MemVerge/nf-float@0.1.7...0.1.8
Enable the plugin to read credentials from the environment variable:
Hide the password in the log.
Add samples in the README to illustrate how to input the credentials with NextFlow secrets.