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-16606: ap_pipe should not create DB automatically #39
Conversation
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, few minor comments.
@@ -161,7 +163,7 @@ A full command looks like | |||
|
|||
.. prompt:: bash | |||
|
|||
ap_pipe.py repo --calib repo/calibs --rerun processed -C $AP_PIPE_DIR/config/calexpTemplates.py -c associator.level1_db.db_name=ppdb/association.db --id visit=123456 ccdnum=42 filter=g --template /path/to/calexp/templates --templateId visit=234567 | |||
ap_pipe.py repo --calib repo/calibs --rerun processed -C $AP_PIPE_DIR/config/calexpTemplates.py -c ppdb.isolation_level=READ_UNCOMMITTED -c ppdb.db_url="sqlite:///ppdb/association.db" --id visit=123456 ccdnum=42 filter=g --template /path/to/calexp/templates --templateId visit=234567 |
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.
General comment - it would be nice to get rid of tat nuisance parameter ppdb.isolation_level=READ_UNCOMMITTED
and have more standard defaults in configuration. Maybe not now, but something for the new ticket.
5084f40
to
5894ec6
Compare
This is a breaking change to the command-line interface.
5894ec6
to
064edbd
Compare
This PR creates a
make_ppdb.py
script that takes the same config arguments asap_pipe.py
and only callsPpdb.makeSchema
, then removes the corresponding call fromApPipeTask
*. This gives users more control over database creation and avoids certain race conditions whenApPipeTask
is run in parallel.*For backward-compatibility, the call from
ApPipeTask
may be reinstated and scheduled for removal on another ticket. See discussion on#dm-alert-prod
.