Pros:
- Less duplication
- Sphinx shows methods docstring on the client API as well
Cons:
- Some Python magic
- Won't play well with static analysis tools.
Depends on D507
Paths
| Differential D505 Authored by vlorentz on Oct 5 2018, 6:25 PM.
Details
Diff Detail
Event TimelineHerald added a reviewer: Reviewers. · View Herald TranscriptOct 5 2018, 6:25 PM2018-10-05 18:25:57 (UTC+2) Harbormaster completed remote builds in B1555: Diff 1497.Oct 5 2018, 6:25 PM2018-10-05 18:25:58 (UTC+2) This comment was removed by vlorentz. Harbormaster completed remote builds in B1556: Diff 1498.Oct 5 2018, 6:32 PM2018-10-05 18:32:10 (UTC+2) Comment Actions I'm not sure I'm a huge fan of some of the hacks required (removing the cur/self/... arguments), but on the purely technical side this looks very good. The only thing I would question is whether it would be worth it to start directly by factorizing that in the RPC itself, in order to force ourselves finding generic solutions that would work for the rest of the remote APIs? Comment Actions
How else can we avoid this though?
People might as well. Also, tests are fine! So that's a good sign it does what i think it does. I don't think it's clear to me what happens during import and the implications here (since we run code here to declare methods/functions/class, instead of simply declaring those). In that regard, that might be a clue to push further below that kind of code...
... (as seirl hinted at)... Cheers,
vlorentz added inline comments. Harbormaster completed remote builds in B1577: Diff 1537.Oct 8 2018, 1:52 PM2018-10-08 13:52:19 (UTC+2)
Comment Actions
Indeed. I just moved all the code to swh.core.api and it looks even nicer this way (no need to put the for loop at the root of a module). I'll write a Diff later today. Harbormaster completed remote builds in B1603: Diff 1571.Oct 9 2018, 5:29 PM2018-10-09 17:29:03 (UTC+2) vlorentz added a child revision: D507: Add functions to generate HTTP API clients and servers from databases..Oct 9 2018, 5:29 PM2018-10-09 17:29:36 (UTC+2) vlorentz removed a child revision: D507: Add functions to generate HTTP API clients and servers from databases.. ardumont edited the summary of this revision. (Show Details)Oct 11 2018, 9:37 AM2018-10-11 09:37:02 (UTC+2) ardumont added a parent revision: D507: Add functions to generate HTTP API clients and servers from databases.. Harbormaster completed remote builds in B1620: Diff 1594.Oct 11 2018, 10:27 AM2018-10-11 10:27:57 (UTC+2) Comment Actions In this current state, i have errors on make test, the base of which seems to be this: TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases steps to reproduce: cd swh-core arc patch D507 cd ../swh-indexer arc patch D505 make test Comment Actions
Details are at P312 Harbormaster completed remote builds in B1625: Diff 1600.Oct 11 2018, 11:29 AM2018-10-11 11:29:20 (UTC+2) This revision is now accepted and ready to land.Oct 11 2018, 11:43 AM2018-10-11 11:43:05 (UTC+2) Closed by commit rDCIDXbdbb8e0864ed: Autogenerate the Indexer Storage HTTP API. (authored by vlorentz). · Explain WhyOct 11 2018, 11:44 AM2018-10-11 11:44:48 (UTC+2) This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 1594 swh/indexer/storage/__init__.py
swh/indexer/storage/api/client.py
swh/indexer/storage/api/server.py
|
Don't we want to keep the original docstring as well here (@functools.wraps)?