Page MenuHomeSoftware Heritage

Make db_transaction* remove db/cur from the signature.
ClosedPublic

Authored by vlorentz on Jan 24 2020, 5:45 PM.

Details

Summary

Rremoving them allows testing the function's signature
matches the existing signature of a specification and type checking.

Moreover, they should not be used by users of the class, so there is no
reason to have them appear in the documentation (generated from
the signature).

Depends on D2585.

Diff Detail

Repository
rDCORE Foundations and core functionalities
Branch
db_transaction_signature
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10295
Build 15291: tox-on-jenkinsJenkins
Build 15290: arc lint + arc unit

Event Timeline

ardumont added a subscriber: ardumont.

I agree with the reasoning in the description.

As it's in core though, I'm unsure about the impacts so if someone else could review as well, that'd be neat.

Note that i'm wondering whether it would not be best to constraint this parameters removal to the storage test defined in your other diff
As far as i could tell, this is for that unique test [1].

[1] https://forge.softwareheritage.org/D2587#inline-17172

swh/core/db/common.py
10

docstring

This revision is now accepted and ready to land.Jan 27 2020, 11:11 AM

As it's in core though, I'm unsure about the impacts so if someone else could review as well, that'd be neat.

Function signatures have no impact on code execution. It only affects code using introspection; which for us is only sphinx.

As it's in core though, I'm unsure about the impacts so if someone else could review as well, that'd be neat.

Function signatures have no impact on code execution. It only affects code using introspection; which for us is only sphinx.

https://jenkins.softwareheritage.org/job/DCIDX/job/tests/816/console build disagrees.
As far as i could tell, as we are piling other decorators, that somehow makes it fail.
Removing the remove_kwargs decorator (or avoiding messing with f.signature) and everythings get back to its feet.

Yes, apparently, Flask also inspects signatures.

I'll fix the issue in swh-indexer