Page MenuHomeSoftware Heritage

Fix directory_add to actually insert the manifest + add directory_get_raw_manifest
ClosedPublic

Authored by vlorentz on Jan 24 2022, 12:52 PM.

Details

Summary

I don't expect directory_get_raw_manifest to be used, but it is needed for tests,
so why not.

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D7024 (id=25455)

Rebasing onto 5874905742...

Current branch diff-target is up to date.
Changes applied before test
commit 2f364cd7c5a596d9957b3353673d282cc816c28c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 24 12:52:04 2022 +0100

    Fix directory_add to actually insert the manifest + add directory_get_raw_manifest
    
    I don't expect directory_get_raw_manifest to be used, but it is needed for tests,
    so why not.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1540/ for more details.

Oof. I guess that's one more reason to be wary of TODOs in tests.

There's a few comments but no show stopper that I can see.

Either way, I guess the vault will want to use directory_get_raw_manifest eventually, so we'd have had to implement this?

swh/storage/cassandra/cql.py
659–662

How did that get here?

691

"the row (with metadata)? from the main directory table" ?

swh/storage/in_memory.py
301

ReleaseRow?

swh/storage/postgresql/storage.py
523–524

s/ids/metadata/

This revision is now accepted and ready to land.Jan 24 2022, 1:32 PM
vlorentz marked 2 inline comments as done.

apply some comments

swh/storage/cassandra/cql.py
659–662

I replaced str with Sha1Git while I was at it

691

"metadata" is an overloaded term :/

Build is green

Patch application report for D7024 (id=25456)

Rebasing onto 5874905742...

Current branch diff-target is up to date.
Changes applied before test
commit 8e2921944a7c30a1acc3d8f7e6b5e75f4dc59eaf
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 24 12:52:04 2022 +0100

    Fix directory_add to actually insert the manifest + add directory_get_raw_manifest
    
    I don't expect directory_get_raw_manifest to be used, but it is needed for tests,
    so why not.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1541/ for more details.

swh/storage/cassandra/cql.py
691

I know, but I had to think a lot about what a "main directory row" would be :-)

Maybe just say "Return fields from the main directory table (e.g. raw_manifest)"?

vlorentz marked 2 inline comments as done.

docstring

This revision was landed with ongoing or failed builds.Jan 25 2022, 10:46 AM
This revision was automatically updated to reflect the committed changes.

Build is green

Patch application report for D7024 (id=25482)

Rebasing onto 6f0252465c...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-1542-D7024.
Changes applied before test

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1542/ for more details.