Page MenuHomeSoftware Heritage

Deduplicate server code.
ClosedPublic

Authored by vlorentz on Dec 12 2019, 7:02 PM.

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Dec 12 2019, 7:02 PM
anlambert accepted this revision.Dec 13 2019, 5:03 PM
anlambert added a subscriber: anlambert.

Looks good too !

This revision is now accepted and ready to land.Dec 13 2019, 5:03 PM

Note that this diff does more than just deduplicate code:

  • it installs the full RPCServerApp application and use its factory to generate what we manually installed as route endpoints (similarly as the indexer storage did a while back, with no issue that we know ;)

    -> doing so though, it drops the metrics computations on those manually installed endpoints (server module)

    -> and moves it on the storage.py module. which i don't really know if it's good or bad (it's not deduplication).
  • moves the metrics functions into its own module (which is good but this had nothing to do with a deduplication (so a commit aside would be best)

I'm not against this.
My point is more that this should be clearly stated in the description's diff (as a tradeoff, deduplicate server code and moves metrics computations in storage.py module or some such)

Also as the diff is quite big, the module separation metrics should have been in another commit and diff.
That would have reduced a bit that diff (not much though).

swh/storage/api/server.py
40 ↗(On Diff #8648)

This seems to have become irrelevant in this diff (i don't see it used anymore).

vlorentz updated this revision to Diff 8679.Dec 16 2019, 1:34 PM

apply @ardumont's comments