-
Notifications
You must be signed in to change notification settings - Fork 21
Caid refactor #927
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
Caid refactor #927
Conversation
@@ -106,7 +104,10 @@ def register_alleles( | |||
|
|||
|
|||
def build_url(base_url: str, reference_genome: ReferenceGenome) -> str: | |||
login, password = get_ar_credentials_from_secret_manager() | |||
login, password = ( | |||
Env.CLINGEN_ALLELE_REGISTRY_LOGIN, |
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.
are these just going to be configured in plain text in helm?
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.
The LOGIN
yes, the PASSWORD
is going to be fetched from the kubernetes secret https://github.com/broadinstitute/seqr-helm/pull/71/files#diff-a43f3d84b733a2ff1c60f1dc01111db8fe8c957fa5c6d097b129147549977819R4. (we might put the login in the secret too just for sanity... cause that's what we're doing now?)
The idea is to initially move the secret fetch to airflow
(so it'll be passed in plain text for the next few weeks 🤦 ) and then eventually it'll go away.
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.
The airflow side is here: https://github.com/broadinstitute/seqr-pipeline-airflow/pull/297
No description provided.