Page MenuHomeSoftware Heritage
Differential D7952

Add type annotation for JournalWriter related code
ClosedPublic
Actions

Authored by douardda on Jun 3 2022, 10:14 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDJNL903f8e601aac: Add type annotation for JournalWriter related code
rDJNL908f015465a3: Remove unused 'privileged' argument from journal writer's write_addition(s)
Summary
  • add a JournalWriterInterface in a new interface.py module,
  • add a flush() method to this interface,
  • remove (now unused) write_update methods,
  • move all type-related code in this module,
  • fix InMemoryJournalWriter type annotations.
  • remove unused 'privileged' argument from journal writer's write_addition(s) for StreamJournalWriter and InMemoryJournalWriter, and make them consistent with KafkaJournalWriter.

Diff Detail

Event Timeline

douardda created this revision.Jun 3 2022, 10:14 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 3 2022, 10:14 AM
swh-public-ci added a comment.Jun 3 2022, 10:15 AM

Build has FAILED

Patch application report for D7952 (id=28637)

Rebasing onto d1b7ad5d9d...

Current branch diff-target is up to date.
Changes applied before test
commit 52398ad80336a3d850ca8e7264b4ce20d5c34d89
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 2 17:17:39 2022 +0200

    Add type annotation for JournalWriter related code
    
    - add a JournalWriterInterface in a new interface.py module,
    - add a flush() method to this interface,
    - remove (now unused) write_update methods,
    - move all type-related code in this module,
    - fix InMemoryJournalWriter type annotations.

commit db9d20294153430aad0975ecb67d50732fbf7699
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jun 3 09:46:16 2022 +0200

    Remove unused 'privileged' argument from journal writer's write_addition(s)
    
    for StreamJournalWriter and InMemoryJournalWriter, and make them
    consistent with KafkaJournalWriter.

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/209/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/209/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 3 2022, 10:15 AM
Harbormaster failed remote builds in B29720: Diff 28637!
douardda mentioned this in D7933: add a kafka_stream_to_value helper function in serializers.py.Jun 3 2022, 10:15 AM
douardda added a child revision: D7933: add a kafka_stream_to_value helper function in serializers.py.Jun 3 2022, 10:16 AM
douardda updated this revision to Diff 28639.Jun 3 2022, 10:28 AM

Small fixes and update copyrights

swh-public-ci added a comment.Jun 3 2022, 10:33 AM

Build is green

Patch application report for D7952 (id=28639)

Rebasing onto d1b7ad5d9d...

Current branch diff-target is up to date.
Changes applied before test
commit 5b91758fd5f7691946d73fa93e0f4e7123cba2a0
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 2 17:17:39 2022 +0200

    Add type annotation for JournalWriter related code
    
    - add a JournalWriterInterface in a new interface.py module,
    - add a flush() method to this interface,
    - remove (now unused) write_update methods,
    - move all type-related code in this module,
    - fix InMemoryJournalWriter type annotations.

commit 908f015465a3bcd20b0701e7ec80ca4d5cb0b223
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jun 3 09:46:16 2022 +0200

    Remove unused 'privileged' argument from journal writer's write_addition(s)
    
    for StreamJournalWriter and InMemoryJournalWriter, and make them
    consistent with KafkaJournalWriter.

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

Harbormaster completed remote builds in B29722: Diff 28639.Jun 3 2022, 10:33 AM
douardda requested review of this revision.Jun 3 2022, 10:33 AM
vlorentz added a subscriber: vlorentz.Jun 3 2022, 10:48 AM

I think I would rather keep the TypeVar definition in each module that needs it (assume it is still actually needed).

It is a Python quirk that type variables can be imported; other languages normally scope them to a class or function declaration.

swh/journal/writer/interface.py
31–37

no need for the typevar here; they could only be useful if you needed to parametrize the whole class, or express that the return value has the same type as the argument

douardda updated this revision to Diff 28649.Jun 3 2022, 12:03 PM

remove TValue typevar as suggested by vlorentz

douardda marked an inline comment as done.Jun 3 2022, 12:04 PM
swh-public-ci added a comment.Jun 3 2022, 12:08 PM

Build has FAILED

Patch application report for D7952 (id=28649)

Rebasing onto d1b7ad5d9d...

Current branch diff-target is up to date.
Changes applied before test
commit 2d2c6a7dc487e2a7741a7a7652225fcf90d6d7f9
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 2 17:17:39 2022 +0200

    Add type annotation for JournalWriter related code
    
    - add a JournalWriterInterface in a new interface.py module,
    - add a flush() method to this interface,
    - remove (now unused) write_update methods,
    - move all type-related code in this module,
    - fix InMemoryJournalWriter type annotations,
    - remove usage of the TValue TypeVar in favor of using ValueProtocol
      directly.
    
    foldme

commit 908f015465a3bcd20b0701e7ec80ca4d5cb0b223
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jun 3 09:46:16 2022 +0200

    Remove unused 'privileged' argument from journal writer's write_addition(s)
    
    for StreamJournalWriter and InMemoryJournalWriter, and make them
    consistent with KafkaJournalWriter.

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/213/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/213/console

Harbormaster failed remote builds in B29732: Diff 28649!Jun 3 2022, 12:08 PM
douardda updated this revision to Diff 28652.Jun 3 2022, 12:19 PM

fix

douardda updated this revision to Diff 28654.Jun 3 2022, 12:23 PM

actual fix

swh-public-ci added a comment.Jun 3 2022, 12:24 PM

Build has FAILED

Patch application report for D7952 (id=28652)

Rebasing onto d1b7ad5d9d...

Current branch diff-target is up to date.
Changes applied before test
commit 2d2c6a7dc487e2a7741a7a7652225fcf90d6d7f9
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 2 17:17:39 2022 +0200

    Add type annotation for JournalWriter related code
    
    - add a JournalWriterInterface in a new interface.py module,
    - add a flush() method to this interface,
    - remove (now unused) write_update methods,
    - move all type-related code in this module,
    - fix InMemoryJournalWriter type annotations,
    - remove usage of the TValue TypeVar in favor of using ValueProtocol
      directly.
    
    foldme

commit 908f015465a3bcd20b0701e7ec80ca4d5cb0b223
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jun 3 09:46:16 2022 +0200

    Remove unused 'privileged' argument from journal writer's write_addition(s)
    
    for StreamJournalWriter and InMemoryJournalWriter, and make them
    consistent with KafkaJournalWriter.

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/215/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/215/console

Harbormaster failed remote builds in B29735: Diff 28652!Jun 3 2022, 12:24 PM
swh-public-ci added a comment.Jun 3 2022, 12:28 PM

Build is green

Patch application report for D7952 (id=28654)

Rebasing onto d1b7ad5d9d...

Current branch diff-target is up to date.
Changes applied before test
commit 903f8e601aacaaf2d9326a4d7e2595ed3098913c
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 2 17:17:39 2022 +0200

    Add type annotation for JournalWriter related code
    
    - add a JournalWriterInterface in a new interface.py module,
    - add a flush() method to this interface,
    - remove (now unused) write_update methods,
    - move all type-related code in this module,
    - fix InMemoryJournalWriter type annotations,
    - remove usage of the TValue TypeVar in favor of using ValueProtocol
      directly.

commit 908f015465a3bcd20b0701e7ec80ca4d5cb0b223
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jun 3 09:46:16 2022 +0200

    Remove unused 'privileged' argument from journal writer's write_addition(s)
    
    for StreamJournalWriter and InMemoryJournalWriter, and make them
    consistent with KafkaJournalWriter.

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

Harbormaster completed remote builds in B29737: Diff 28654.Jun 3 2022, 12:28 PM
vlorentz accepted this revision.Jun 7 2022, 10:50 AM
This revision is now accepted and ready to land.Jun 7 2022, 10:50 AM
Closed by commit rDJNL908f015465a3: Remove unused 'privileged' argument from journal writer's write_addition(s) (authored by douardda). · Explain WhyJun 7 2022, 11:55 AM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDJNL908f015465a3: Remove unused 'privileged' argument from journal writer's write_addition(s).
douardda added a commit: rDJNL903f8e601aac: Add type annotation for JournalWriter related code.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
journal/
tests/
31 lines100%
Loading...
writer/
38 lines67%
Loading...
28 lines95%
Loading...
41 lines70%
Loading...
12 lines95%
Loading...
24 lines94%
Loading...

Diff 28639

View Options

swh/journal/tests/test_inmemory.py

Loading...
View Options

swh/journal/writer/__init__.py

Loading...
View Options

swh/journal/writer/inmemory.py

Loading...
View Options

swh/journal/writer/interface.py

Loading...
View Options

swh/journal/writer/kafka.py

Loading...
View Options

swh/journal/writer/stream.py

Loading...
About · Developer information · Legal