Page MenuHomeSoftware Heritage

Split OriginIndexer.run into smaller methods.
ClosedPublic

Authored by vlorentz on Feb 6 2019, 4:50 PM.

Details

Summary

So subclasses can override them if needed.

Diff Detail

Repository
rDCIDX Object indexer
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.Feb 6 2019, 4:50 PM
vlorentz updated this revision to Diff 3419.Feb 6 2019, 4:52 PM
  • Fix typo.
douardda requested changes to this revision.Feb 7 2019, 9:58 AM
douardda added a subscriber: douardda.

One drawback of this function extraction is that you now have to keep an intermediate result (the origins list). Couldn't this be avoided using generators?

swh/indexer/indexer.py
555

self not being used in the body of the method, make it either a simple function or a static method. Also extracting a piece of code like this in a dedicated function make it easier to test it. Just sayin'!

This revision now requires changes to proceed.Feb 7 2019, 9:58 AM

One drawback of this function extraction is that you now have to keep an intermediate result (the origins list). Couldn't this be avoided using generators?

That's a very small list (I'd be surprised if it ever gets over 10KB). And starting with D1092, we'll download the whole list at once anyway.

vlorentz updated this revision to Diff 3429.Feb 7 2019, 10:48 AM
  • Move origin_get_params outside a class.
vlorentz marked an inline comment as done.Feb 7 2019, 10:48 AM

One drawback of this function extraction is that you now have to keep an intermediate result (the origins list). Couldn't this be avoided using generators?

That's a very small list (I'd be surprised if it ever gets over 10KB). And starting with D1092, we'll download the whole list at once anyway.

Well, you do not know that for sure, the list is an input for this method. And to state the obvious, oceans are made of small drops ;-)

Whatever, it's probably not a deal, indeed, so let's keep moving...

douardda accepted this revision.Feb 7 2019, 5:45 PM
This revision is now accepted and ready to land.Feb 7 2019, 5:45 PM
vlorentz updated this revision to Diff 3474.Feb 8 2019, 10:27 AM
  • rebase
This revision was automatically updated to reflect the committed changes.