This moves the interesting parts of D505 into the core, so other components can use them as well.
Details
- Reviewers
ardumont seirl - Group Reviewers
Reviewers - Commits
- rDCOREdefc4f322e6e: Add functions to generate HTTP API clients and servers from databases.
make test
Diff Detail
- Repository
- rDCORE Foundations and core functionalities
- Branch
- http-api-generator
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 1605 Build 1949: arc lint + arc unit
Event Timeline
While I'm stubborn on this, I would have liked to see this commit splitted in 2 (server side / client side)
swh/core/api.py | ||
---|---|---|
23–29 | not sure I get the reason for this to be a class. IMHO it should be a plain decorator, the class here seems to be just a namespace... and I tend to avoid unnecessary inheritance, and this looks to me as a source of unnecessary class heritage. |
swh/core/api.py | ||
---|---|---|
23–29 | The idea is that in the future we may need to extend it; but I don't have a strong opinion one way or the other. |
swh/core/api.py | ||
---|---|---|
23–29 | Here is a clue, if we do not need it right now, keep the original one (function as [1]). And indeed, the function name endpoint becomes less clear, remote_endpoint seems better. Also, note that this pattern of remote api is not necessarily for storage api (cf. vault, scheduler). [1] https://forge.softwareheritage.org/D505?vs=on&id=1498&whitespace=ignore-most#inline-2433 | |
117 | Mention in the docstrings the implementations example so that when the documentation is generated, we can follow those samples. See for example :class:`IndexerStorage`, :class:`TestStorage`, etc... | |
119 | Mention IndexerStorage as an example... I'd change the sentence to: | |
195 | Mention what the args are. |
fwiw, i'm waiting on the metaclass implementation since the discussion conclusion converged on this:
irc discussion:
09:29:30 douardda_: vlorentz: I wonder if a metaclass would not have been simpler there D507 09:29:31 -- Notice(swhbot): D507 (author: vlorentz, Needs Review) on swh-core: Add functions to generate HTTP API clients and servers from databases. <https://forge.softwareheritage.org/D507> 09:31:49 vlorentz: douardda_: a metaclass can't take an argument 09:32:10 vlorentz: and we need to pass the Storage class somehow 09:45:45 douardda_: a decorated metaclass then! (just kidding) 09:46:16 vlorentz: You mean a metaclass factory 09:47:10 vlorentz: A metametaclass would be the "right" way to do it, but ugh 09:49:36 douardda_: ;) 11:24:27 +seirl: vlorentz: huh? you can totally do MyClass(metaclass=MyMeta, arg1=42), unless I'm misunderstanding what you're saying 11:24:36 vlorentz: seirl: ooooh 11:24:46 vlorentz: seirl: indeed, I totally forgot that 11:25:07 +seirl: it's easier in 3.6 but it works well in 3.x 11:25:10 vlorentz: I guess I'll make it a metaclass then
Cheers,
swh/core/tests/test_api.py | ||
---|---|---|
9 | You need to rebase your diff to the latest swh-core. |
Add dependency on requests-mock.
I forgot we need to add the dependency on debian/control as well.