-
Notifications
You must be signed in to change notification settings - Fork 1
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 containers #41
Conversation
5f83aa7
to
1695839
Compare
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.
Really nice in general, made some comments, mostly grammatical!
34d6595
to
39d706b
Compare
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.
Looks good! The docker setup runs well when I tested. I just left a few minor comments.
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.
Some changes break functionality, take a look at comments.
sda-admin
needs to change as well to be compatible with the new images (this is just changing the default MQ_QUEUE_PREFIX
to ``sdaon line 14 and
accessionIDs` to `accession` on line 505.
128c8a3
to
334cf79
Compare
Otherwise we don't get the correct KID in the token.
4d04b1e
to
b55600b
Compare
Set correct queue/routingkey for accessions and fix general semantics.
b55600b
to
4579afd
Compare
Looks reasonable, but the new changes seem to have brought in a new error regarding scopes when |
1d44798
to
624d08e
Compare
624d08e
to
6673638
Compare
I still can't get this to run. Even after while Maybe we could try a pair debug next week. |
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.
Last changes did the trick, works great now.
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.
Looks good!
Some minor things need be added to the instructions for sda-admin
usage.
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.
🥳
Co-authored-by: Alex Aperis <[email protected]>
**Note: this requires the
deployment-fixes
branch ofGenomicDataInfrastructure/starter-kit-lsaai-mock
unless that have been merged.sensitive-data-archive
sda
componentscloses #40