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-43416: Migrate AP code to external APDB configs #224

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Conversation

kfindeisen
Copy link
Member

This PR modifies the ap_verify framework and the metrics pipelines to use new-style configs instead of handling ApdbConfig objects directly. It must be merged after lsst/ap_association#210 and lsst/verify#124.


  • Do unit tests pass (scons and/or stack-os-matrix)?
  • Did you run ap_verify.py on at least one of the standard datasets?
    For changes to metrics, the print_metricvalues script from lsst.verify will be useful.

ApdbMetricTasks should have their APDB configured in a way that's
consistent with diaPipe, or they will break (possibly only at runtime).
None of the tests actually try to load an APDB, so there's no need
to define apdb_config_url for those components.
This field specifies where to store the config created by ap_verify.
Copy link
Contributor

@isullivan isullivan 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, very minor comment.

python/lsst/ap/verify/pipeline_driver.py Outdated Show resolved Hide resolved
This change removes ap_verify's dependency on the deprecated makeApdb
function. Although the new configs allow better support for Cassandra
APDBs, ap_verify itself only supports SQL (particularly SQLite), given
its restricted scope.
@kfindeisen kfindeisen merged commit c735040 into main Apr 30, 2024
2 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-43416 branch April 30, 2024 20:56
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