Page MenuHomeSoftware Heritage
Differential D1993

journal_writer: use journal writer from swh.journal
ClosedPublic
Actions

Authored by douardda on Sep 17 2019, 11:42 AM.

Details

Reviewers
vlorentz
zack
Group Reviewers
Reviewers
Commits
rDSTOC9aa6b9d2b100: journal_writer: use journal writer from swh.journal
rDSTO9aa6b9d2b100: journal_writer: use journal writer from swh.journal
Summary

Move implementation of the InMemoryJournalWriter class and get_journal_writer
function in swh.journal to prevent a dependency loop.

Depends on D1992

Diff Detail

Repository
rDSTO Storage manager
Branch
master
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 7960
Build 11468: tox-on-jenkinsJenkins
Build 11467: arc lint + arc unit

Event Timeline

douardda created this revision.Sep 17 2019, 11:42 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 17 2019, 11:42 AM
swh-public-ci added a comment.Sep 17 2019, 11:43 AM

Build has FAILED

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

Harbormaster failed remote builds in B7815: Diff 6715!Sep 17 2019, 11:43 AM
vlorentz accepted this revision.Sep 17 2019, 11:47 AM
vlorentz added a subscriber: vlorentz.

Make sure the tests pass before landing this diff

This revision is now accepted and ready to land.Sep 17 2019, 11:47 AM
douardda updated this revision to Diff 6717.Sep 17 2019, 1:48 PM

also update test_api_client.py

swh-public-ci added a comment.Sep 17 2019, 1:49 PM

Build has FAILED

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

Harbormaster failed remote builds in B7817: Diff 6717!Sep 17 2019, 1:49 PM
swh-public-ci added a comment.Sep 19 2019, 12:02 PM

Build was aborted

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

zack added a child revision: D2023: typing: minimal changes to make a no-op mypy run pass.Sep 22 2019, 4:13 PM
zack removed a child revision: D2023: typing: minimal changes to make a no-op mypy run pass.Sep 22 2019, 4:48 PM
zack mentioned this in D2023: typing: minimal changes to make a no-op mypy run pass.
zack mentioned this in D2024: typing: minimal changes to make a no-op mypy run pass.Sep 22 2019, 4:51 PM
zack added a subscriber: zack.Sep 22 2019, 4:55 PM
zack added inline comments.
swh/storage/journal_writer.py
0

so, by doing this, we fully embrace an import-time dependency loop between swh-storage and swh-journal, do we really want to go there?

douardda added inline comments.Sep 23 2019, 10:12 AM
swh/storage/journal_writer.py
0

Not sure what you mean here.

Now, since the journal and the storage are the only swh packages currently using this get_journal_writer function, we probably can get rid of this bw-compat module.

zack added inline comments.Sep 23 2019, 10:23 AM
swh/storage/journal_writer.py
0

Not sure what you mean here.

swh-journal has a non-conditional import of swh.journal, and with this change swh-journal will grow a non-conditional import of swh.storage.

Now, since the journal and the storage are the only swh packages currently using this get_journal_writer function, we probably can get rid of this bw-compat module.

It would be great to do so without ever landing the non-conditional import loop mentioned above.

zack requested changes to this revision.Sep 23 2019, 6:52 PM

as discussed f2f, we'll drop the backward compatibility layer as to break the import loop

This revision now requires changes to proceed.Sep 23 2019, 6:52 PM
zack added a child revision: D2023: typing: minimal changes to make a no-op mypy run pass.Sep 24 2019, 1:27 PM
douardda updated this revision to Diff 6838.Sep 27 2019, 11:10 AM

remove journal_writer.py

vlorentz added inline comments.Sep 27 2019, 11:36 AM
swh/storage/tests/test_api_client.py
13

swh.journal should also be in the test requirements

vlorentz added inline comments.Sep 27 2019, 11:39 AM
swh/storage/tests/test_api_client.py
13

nvm you did it already

zack requested changes to this revision.Sep 27 2019, 11:41 AM
zack added inline comments.
swh/storage/tests/test_api_client.py
13

can you make this import conditional for tests too, as you did for the non-test code?

This revision now requires changes to proceed.Sep 27 2019, 11:41 AM
douardda added inline comments.Sep 27 2019, 1:16 PM
swh/storage/tests/test_api_client.py
13

I think not, the test dependency being declared in the requirements-test.txt file.

zack accepted this revision.Sep 27 2019, 1:27 PM
This revision is now accepted and ready to land.Sep 27 2019, 1:27 PM
This revision was landed with ongoing or failed builds.Sep 27 2019, 1:31 PM
Closed by commit rDSTO9aa6b9d2b100: journal_writer: use journal writer from swh.journal (authored by douardda). · Explain Why
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDSTO9aa6b9d2b100: journal_writer: use journal writer from swh.journal.
swh-public-ci added a comment.Sep 27 2019, 2:30 PM

Build was aborted

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

Harbormaster failed remote builds in B7960: Diff 6838!Sep 27 2019, 2:30 PM
douardda added a commit: rDSTOC9aa6b9d2b100: journal_writer: use journal writer from swh.journal.Oct 30 2019, 5:21 PM

Revision Contents
Changeset List

Diff 6838

View Options

requirements-swh-journal.txt

Loading...
View Options

setup.py

Loading...
View Options

swh/storage/in_memory.py

Loading...
View Options

swh/storage/journal_writer.py

Loading...
View Options

swh/storage/storage.py

Loading...
View Options

swh/storage/tests/test_api_client.py

Loading...
About · Developer information · Legal