Active Operations

Operation 4 Land Revision
Operation encountered an error.

Details

Reviewers

Group Reviewers

Commits

rDSTOC823465ff49b6: Remove SQLisms from the tests and API.
R65:823465ff49b6: Remove SQLisms from the tests and API.
rDSTO823465ff49b6: Remove SQLisms from the tests and API.

Diff Detail

Repository

rDSTO Storage manager

Branch

tests-non-pg

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 2374
Build 2899: tox-on-jenkins	Jenkins
Build 2898: arc lint + arc unit

Event Timeline

vlorentz created this revision.Nov 9 2018, 12:06 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptNov 9 2018, 12:06 PM

vlorentz retitled this revision from Remove SQLisms from the tests. to Remove SQLisms from the tests and API..Nov 9 2018, 12:06 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/49/ for more details.

Harbormaster completed remote builds in B2289: Diff 1990.Nov 9 2018, 12:11 PM

vlorentz added a child revision: D645: Add in-memory storage..Nov 9 2018, 12:58 PM

vlorentz mentioned this in D635: [WIP] In-memory storage implementation.Nov 9 2018, 1:01 PM

Thanks, these changes look fine save for a few comments inline

swh/storage/storage.py
163	Can we extract the key that has collided? The `diag` attribute on the exception should have the info http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.Diagnostics
1216–1217	The keys should match what's in the stored procedure for stat_counters (so, content, skipped_content, directory, revision, release, snapshot, origin, origin_visit, and maybe person; directory_entry_dir was removed because it's meaningless)
swh/storage/tests/test_storage.py
571	Also check that the key got extracted ?

This revision now requires changes to proceed.Nov 9 2018, 2:03 PM

Fix column names in refresh_stat_counters.

swh/storage/storage.py
163	The best info I can get from that attribute is the `message_detail`, which is `'Key (sha1)=(\\x34973274ccef6ab4dfaaf86599792fa9c3fe4689) already exists.'`. I could parse it, but eww

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/57/ for more details.

Harbormaster completed remote builds in B2317: Diff 2008.Nov 9 2018, 6:09 PM

vlorentz mentioned this in D645: Add in-memory storage..Nov 9 2018, 6:38 PM

vlorentz marked an inline comment as done.Nov 14 2018, 11:02 AM

olasd added inline comments.Nov 14 2018, 2:21 PM

swh/storage/storage.py
163	You can check: `e.diag.sqlstate == '23505'` (unique constraint failure - documented on https://www.postgresql.org/docs/10/errcodes-appendix.html) `e.diag.table_name == 'content'` and use the `e.diag.constraint_name` attribute as a mapping: `content_pkey` -> (currently) duplicate sha1 `content_XXX_idx` -> duplicate XXX (with XXX in sha1_git, sha256) We don't enforce distinct blake2s256 hashes (we should also probably drop the sha256 enforcement). Anything else should bubble up as the original IntegrityError exception.

vlorentz marked an inline comment as done.Nov 14 2018, 2:45 PM

vlorentz added inline comments.

swh/storage/storage.py
163	`content_XXX_idx` -> duplicate XXX (with XXX in sha1_git, sha256) `grep "_idx" swh/storage/sql/*.sql` shows no result.

vlorentz marked an inline comment as done.Nov 14 2018, 2:46 PM

vlorentz added inline comments.

swh/storage/storage.py
163	nvm, I misunderstood

Give the name of the colliding hash when raising HashCollision.

Harbormaster failed remote builds in B2361: Diff 2044!Nov 14 2018, 2:47 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/70/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/70/console

In D642#13319, @swh-public-ci wrote:

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/70/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/70/console

Looks like this needs a rebase