Page MenuHomeSoftware Heritage

Add type annotation for JournalWriter related code
ClosedPublic

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

Details

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

Repository
rDJNL Journal infrastructure
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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.Fri, Jun 3, 10:15 AM
Harbormaster failed remote builds in B29720: Diff 28637!

Small fixes and update copyrights

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.

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 ↗(On Diff #28639)

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

remove TValue typevar as suggested by vlorentz

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

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

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.

This revision is now accepted and ready to land.Tue, Jun 7, 10:50 AM