-
Notifications
You must be signed in to change notification settings - Fork 20
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
Integrate use of dataregistry into gcr-catalogs #638
Conversation
…between source types. Other minor clean-up
…rce (but only one at a time
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.
Thanks for preparing this @JoanneBogart. I think the proposed change makes sense, and the high-level implementation (such as creating base_config
) is very reasonable, so I am approving this PR. However, I have not read the code line-by-line (especially the new dr_register.py
) and have not tested this PR either. If a detailed review is needed, I can help but it will need to wait until I have more time to attend to it.
Thank you, @yymao ! |
I don't have other tests beyond the existing CI tests. I wonder if it's useful to add a CI test to check the functionality of |
It would be useful but would take some work since the CI workflow has no access to NERSC resources, including the data registry Postgres database. We would have to create a database as part of the workflow and add entries. This kind of thing is done for the dataregistry CI. |
If the dataregistry module is available in the user's environment, they can choose to use it to find and load catalogs instead of the traditional config file mechanism.
fix #637