Skip to content
This repository was archived by the owner on May 31, 2024. It is now read-only.

Conversation

@kankou-aliaksei
Copy link
Contributor

@kankou-aliaksei kankou-aliaksei commented Apr 27, 2022

Description of Changes

[//]: # This adds option to pass custom environment variables into WES Adapter
[//]: # This adds option to pass custom environment variables into Workflow Job

Description of how you validated changes

[//]: # agc account activate --customWesEnvVars "CUSTOM_WORKFLOW_JOB_ENVIRONMENTS=[{ \"name\": \"MINIWDL__AWS__DESCRIBE_PERIOD\", \"value\": 10 }, { \"name\": \"MINIWDL__AWS__SUBMIT_PERIOD\", \"value\": 10 }]"
[//]: # After agc account activate the SSM parameter /agc/_common/customWesEnvVars must be set
[//]: # agc configure email [email protected]
[//]: # After agc context deploy --context miniContext a WES Adapter Lambda must have CUSTOM_WORKFLOW_JOB_ENVIRONMENTS env var
[//]: # After agc workflow run hello --context miniContext go to Batch Workflow Job Logs and find MINIWDL__AWS__DESCRIBE_PERIOD and MINIWDL__AWS__SUBMIT_PERIOD under === ENVIRONMENT === section

Checklist

  • If this change would make any existing documentation invalid, I have included those updates within this PR
  • I have added unit tests that prove my fix is effective or that my feature works
  • I have linted my code before raising the PR
  • Title of this Pull Request follows Conventional Commits standard: https://www.conventionalcommits.org/en/v1.0.0/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@kankou-aliaksei kankou-aliaksei changed the title Added option to pass custom WES Adapter environment variables feat: Added option to pass custom WES Adapter environment variables Apr 27, 2022
Copy link
Contributor

@markjschreiber markjschreiber left a comment

Choose a reason for hiding this comment

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

I like the idea but I wonder why these variables are being set at the account activate level? If you set wes env vars at the level of account activate then these would apply to all WES adapters used even though each engine has it's own WES adapter and potentially each WES adapter could behave very differently. I wonder if it would be better to move the CLI flag to the agc context deploy command? That would let you deploy each context with the ENV flags appropriate to that WES adapter.

@wleepang
Copy link
Contributor

related to #396

@kankou-aliaksei
Copy link
Contributor Author

See #420

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants