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

Update nf-syn23664726-s3.yaml #601

Merged
merged 3 commits into from
Aug 31, 2023
Merged

Conversation

allaway
Copy link
Contributor

@allaway allaway commented Aug 25, 2023

Maybe this will resolve permission errors in Tower?

Maybe this will resolve permission errors in Tower?
@allaway allaway requested review from a team as code owners August 25, 2023 19:00
@@ -21,7 +21,9 @@ parameters:
- 'arn:aws:iam::694704239947:user/giri' ## Nebula Genomics access
- 'arn:aws:iam::969434029702:user/audrisc' ##Stanford access (Audris Chiang)
- 'arn:aws:iam::728882028485:role/ntap-add5-project-TowerForgeBatchHeadJobRole-X9Y068JVFXHC' ## Sage NF Tower access
- 'arn:aws:iam::728882028485:role/ntap-add5-project-TowerForgeBatchWorkJobRole-TYLG8H2QIS7R' ## Sage NF Tower access
- 'arn:aws:iam::728882028485:role/ntap-add5-project-TowerForgeBatchWorkJobRole-TYLG8H2QIS7R' ## Sage NF Tower access
Copy link
Member

Choose a reason for hiding this comment

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

there already a TowerForgeBatchHeadJobRole defined in the line above. is this one intended to replace the one above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @zaro0508, there are three roles I am trying to add: TowerForgeBatchHeadJobRole, TowerForgeBatchWorkJobRole, TowerForgeServiceRole.

My end goal is to use Tower to index a non-Tower bucket (s3://nf-syn23664726-s3-bucket-n9uakf7bowwd/). Do you think that these three roles will be sufficient to do this? I tried just TowerForgeBatchHeadJobRole and TowerForgeBatchWorkJobRole and that didn't appear to work.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry, I don't know Tower well enough to answer your question @allaway. I would suggest asking @thomasyu888

Copy link
Member

@thomasyu888 thomasyu888 Aug 28, 2023

Choose a reason for hiding this comment

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

@zaro0508 I'm actually not certain. @allaway is getting a permission issue, but these are the specific roles that are used when processing data on that tower workspace.

Is the policy set up in a way to allow for indexing of files to a Synapse project via these roles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After talking with Adam, I think I need to make the config change in this PR, but I also think I need to change the config for the nextflow project.

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

LGTM

@zaro0508
Copy link
Member

please fix the pre-commit failures @allaway then we can merge

@allaway
Copy link
Contributor Author

allaway commented Aug 30, 2023

Oops, trailing whitespace. Fixed!

@zaro0508 zaro0508 merged commit 00cdd20 into Sage-Bionetworks:master Aug 31, 2023
3 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.

3 participants