-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add source table #763
Add source table #763
Conversation
6e614dd
to
940ecfb
Compare
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.
Approved, but I think we should maybe test the candidate naming before merging. Would it be feasible to add another image to the tests to make sure this is working ?
Pull Request Test Coverage Report for Build 7938845778Details
💛 - Coveralls |
Proposed way forward: create a clean candidate table and reingest. Rename old real bogus/candidate table. |
@virajkaram I have confirmed that this does work by creating a dummy candidate, which leads to history/the correct name/ndet=2 in database. |
This PR adds a new table for WINTER sources. Fix #732, fix #734. From now onwards:
This is a breaking change, and is somewhat decoupled from the migration of the existing database which we will need to implement. My preference would be to somehow create a clean candidate table and source table, and then reingest old candidates in sequence to assign names correctly. I am just worried it'll undercut all the candidate scanning done so far.