Skip to content
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

FEAT: Add software_license profile #551

Merged
merged 6 commits into from
Sep 18, 2023

Conversation

maxulysse
Copy link
Member

to simplify GHA with Sentieon for pipelines and modules


process {
withLabel: 'sentieon' {
ext.sentieon_auth_mech_base64 = secrets.SENTIEON_AUTH_MECH_BASE64 ?: ''
Copy link

@asp8200 asp8200 Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secrets.SENTIEON_AUTH_MECH_BASE64 ?: '' failed for Ram when secrets.SENTIEON_AUTH_MECH_BASE64 wasn't set. He got

Unknown config secret 'SENTIEON_AUTH_MECH_BASE64'

Copy link

@asp8200 asp8200 Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea is to have many different licenses in conf/nfcore_license.config? 🤔 I wonder if that will work. Say nfcore_license.config refers to nf secrets for tool X and tool Y but the user only has license for X and can only set the nf secrets for X, then it seems to me that this will fail with err msg about missing nf-secrets for tool Y (that is, the same kind of error which we get from running profile test_cache without sentieon test-license nf-secrets set).

Maybe the config should just be for the sentieon test license? Perhaps name it sentieon_license_server_extension.config or sentieon_test_license.config instead of nfcore_license.config?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this should only works when we set up secrets when we start the tests, similarly to how we do it within our CI

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nothing to do with a 'test license' so we can't call it 'test'.

@maxulysse maxulysse changed the title FEAT: Add nfcore_license profile for sentieon FEAT: Add software_license profile Sep 18, 2023
Copy link
Contributor

@sateeshperi sateeshperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@maxulysse maxulysse merged commit 3613585 into nf-core:master Sep 18, 2023
@maxulysse maxulysse deleted the nf_core_sentieon_license branch September 18, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants