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

DM-47882: Refactor config file and add command logging #9

Merged
merged 6 commits into from
Dec 5, 2024

Conversation

fred3m
Copy link
Collaborator

@fred3m fred3m commented Dec 2, 2024

No description provided.

Copy link
Collaborator

@kadrlica kadrlica left a comment

Choose a reason for hiding this comment

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

Looks good. A few comments inline.

credentials_path: /etc/secrets/postgres-credentials.txt
consdb: postgresdb01.cp.lsst.org
butlers:
embargo: s3://embargo@rubin-summit-users/butler.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to deal with the aliasing ourself, or count on the aliases to be setup on the butler configuration? I don't actually know how this is done at the summit and USDF when the worker is deployed, but I know that embargo is mapped to this s3 bucket automagically at the USDF for me. Maybe we want what to setup that alias configuration mechanism instead of defining our own way of aliasing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on Sebastian's comment here, the guess is that the butler alias automagic is also working for the deployed pods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed.

logger.info("Connecting to Butlers")
butlers = {}
for repo_name, repo in config.butlers.items():
butlers[repo_name] = Butler(repo)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want the worker to continue if it can't create the butler?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that's a good point. If something is wrong with only the Butler it would be good for other features to be available.

@fred3m fred3m merged commit b585a90 into main Dec 5, 2024
5 checks passed
@fred3m fred3m deleted the tickets/DM-47882 branch December 5, 2024 15:03
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