Details
- Reviewers
olasd - Group Reviewers
Reviewers - Maniphest Tasks
- T534: Add completion information to softwareheritage.origin_visit table
T538: Add origin_visit API entry points to permit creation/update
Diff Detail
- Repository
- rDSTO Storage manager
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 399 Build 591: Software Heritage Python tests Build 590: arc lint + arc unit
Event Timeline
A few comments inline
| sql/swh-func.sql | ||
|---|---|---|
| 836–852 | I think this function could be done directly in language SQL : remove the declaration, and do return query with [...] returning visit; | |
| swh/storage/tests/test_storage.py | ||
| 1103–1104 | It'd be clearer if the tests were split between the add and update functions. | |
- sql/upgrades/075: 074→075 - Add status info to origin_visit
- Add origin_visit api entry points to create/update origin_visit
| sql/swh-func.sql | ||
|---|---|---|
| 836–852 | Indeed ^^ | |
| swh/storage/tests/test_storage.py | ||
|---|---|---|
| 1103–1104 | I'm on it. | |
- Update occurrence_add api entry point to deal with origin_visit
- Fix origin_visit time policy in occurrence_add api
- Cleanup obsolete reference to date in occurrence_add entry
For consistency with how other object additions are handled, I think we should add a visit column to the occurrence_history temporary table, and use that to populate the "visit" object, rather than pass a new argument to occurrence_add.
Case in point, one of the two new arguments is not being used currently (the origin_id is pulled from the temporary table).
We can revisit this later when we'll have nailed down the so-called snapshot objects, but for now I think consistency trumps any future goals.
| sql/swh-func.sql | ||
|---|---|---|
| 855 | The origin_id is not used here (it's already a column in tmp_occurrence_history). | |
| 866 | We're not pulling any data from origin_visit anymore so you can ditch this join. | |
I think both database upgrades should be merged as 075 is currently breaking stuff that 076 fixes. Apart from that, this looks good.
Thanks!
I think both database upgrades should be merged as 075 is currently breaking stuff that 076 fixes.
Yes, done.