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

RMHDR-200 Use yml config file instead of R script to store pipeline parameters #8

Merged
merged 6 commits into from
Nov 1, 2023

Conversation

pranavanba
Copy link
Collaborator

Depends on #7

Major Changes

  1. Create config.yml file to store pipeline parameters as key: value pairs in yml format
  2. Get config.yml key: value pairs using config::get(config = "prod") and store output in global environment

Minor Changes

  1. Remove references to params.R

@pranavanba pranavanba self-assigned this Oct 27, 2023
@itismeghasyam
Copy link
Contributor

Other than the one issue raised above, this PR looks good to me

@pranavanba
Copy link
Collaborator Author

Other than the one issue raised above, this PR looks good to me

What issue are you referring to @itismeghasyam?

@itismeghasyam
Copy link
Contributor

@pranavanba, the one mentioned in the review. Let me copy paste here.

(for L#6 in config.yml)
I see in the current params.R file, the base key is 'main/parquet/' - an extra '/' in the end. Is it ok to not have it here? (asking as it might cause issues with paste0). Same comment for

PARQUET_BUCKET_BASE_KEY_EXTERNAL
PARQUET_BUCKET_BASE_KEY_ARCHIVE

Copy link

dpulls bot commented Nov 1, 2023

🎉 All dependencies have been resolved !

@pranavanba
Copy link
Collaborator Author

@pranavanba, the one mentioned in the review. Let me copy paste here.

(for L#6 in config.yml) I see in the current params.R file, the base key is 'main/parquet/' - an extra '/' in the end. Is it ok to not have it here? (asking as it might cause issues with paste0). Same comment for

PARQUET_BUCKET_BASE_KEY_EXTERNAL PARQUET_BUCKET_BASE_KEY_ARCHIVE

Thanks for pointing this out Megha. I've updated the config file.

@pranavanba pranavanba merged commit a00f950 into Sage-Bionetworks:main Nov 1, 2023
2 checks passed
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.

2 participants