Prior to this commit, the code was triplicated across the storage backends. Now
all storages use the same collaborator whose concern is writing to the storage.
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Commits
- rDSTO652ecf0aeb68: storages: Refactor journal operations with a dedicated writer collab
tox
Diff Detail
- Repository
- rDSTO Storage manager
- Branch
- dedicate-journal-writer
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 10528 Build 15699: tox-on-jenkins Jenkins Build 15698: arc lint + arc unit
Event Timeline
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/975/ for more details.
I don't like the name writer, it's less explicit. Maybe keep journal_writer?
Also, you could make the journal writer support model objects, so you don't have to use .to_dict() every time?
swh/storage/in_memory.py | ||
---|---|---|
77 | is the evolve still needed? | |
293–296 | Filter before the writer... | |
299–300 | ... and then you can remove this (same for other methods below) |
swh/storage/in_memory.py | ||
---|---|---|
77 | And i thought to address this in your incoming 'validation' diff as well. |
Also, you could make the journal writer support model objects, so you don't have to use .to_dict() every time?
hmmm, i wanted to do that after your diffs to decrease the chances of conflicts.
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/976/ for more details.
Also, you could make the journal writer support model objects, so you don't have to use .to_dict() every time?
hmmm, i wanted to do that after your diffs to decrease the chances of conflicts.
Reflecting a bit again on this, i stand by my statement.
Out storage backend are not yet consistent everywhere in term of dict/model.model.BaseModel objects use.
So that will be a tad annoying to do so right now.
When i do a diff, i try to fix 1 perimeter at a time, not too many at once...
So the code change impacted can be more understandable (also reverted easily if need be).
So here that means, refactor the journal writer related code and that's it.
And i did not change any other logic (so no conversion step, no filtering logic adaptation, etc...).
It's all the more important that i plan to merge this after the validation diff (D2640) take place (which will alter some logic already).
That reduces the surface of code that could conflict.
Cheers,
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/982/ for more details.
- writer: Make the writer able to deal with BaseModel objects
Challenging my own opinion ;)
(i'll be able to tell if i was right or not, i still think i was but let's challenge it)
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/983/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/987/ for more details.
swh/storage/writer.py | ||
---|---|---|
16–28 | You don't need this, swh-journal already handles swh-model objects. |
- Add missing types on remaining collaborator method signatures
- Remove unneeded to_dict conversion as the journal is already swh.model.model.BaseModel aware.
swh/storage/writer.py | ||
---|---|---|
16–28 | Neat. |
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/988/ for more details.
Remove unnecessary copy instruction (already converting from content object to
a new dict to ease the data key removal)
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/989/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/991/ for more details.