[ethPM] Fix bug in package id and release id fetching strategy #1427
Conversation
njgheorghita
force-pushed
the
ethpm-bugfix
branch
from
b87b08e
to
1e227a6
Compare
web3/pm.py
Outdated
| package_ids = [] | ||
| while pointer < num_packages: | ||
| new_ids, pointer = self.registry.functions.getAllPackageIds(pointer, (pointer + BATCH_SIZE)).call() | ||
| package_ids.append(new_ids) |
There was a problem hiding this comment.
@pipermerriam Not sure how you feel about this - feels a bit mutate-y to me, but also the easiest way to implement it. The BATCH_SIZE is also arbitrary, but I tested it locally with values of 1, 2, 4, 10, and 100 and it works fine. I didn't update the tests in this case b/c releasing 100+ packages just to test this case seems a bit overkill.
There was a problem hiding this comment.
This should be able to be converted to a non-mutative solution reasonably easy. IIUC this correctly you need to reverse each of the individual chunks and return in order which I think would be equivalent to just replacing this statement with yield from reversed(new_ids)
njgheorghita
force-pushed
the
ethpm-bugfix
branch
from
1e227a6
to
dba7c73
Compare
web3/pm.py
Outdated
| package_ids = [] | ||
| while pointer < num_packages: | ||
| new_ids, pointer = self.registry.functions.getAllPackageIds(pointer, (pointer + BATCH_SIZE)).call() | ||
| package_ids.append(new_ids) |
There was a problem hiding this comment.
This should be able to be converted to a non-mutative solution reasonably easy. IIUC this correctly you need to reverse each of the individual chunks and return in order which I think would be equivalent to just replacing this statement with yield from reversed(new_ids)
njgheorghita
force-pushed
the
ethpm-bugfix
branch
from
dba7c73
to
a4a7323
Compare
njgheorghita
changed the title
njgheorghita
force-pushed
the
ethpm-bugfix
branch
from
a4a7323
to
293e8d9
Compare
| for release_id in concat([x[::-1] for x in release_ids]): | ||
| yield release_id | ||
| pointer = 0 | ||
| while pointer < num_releases: |
There was a problem hiding this comment.
One more thing. This is probably fine, however, you could save yourself a headache by enforcing that pointer always increases on each loop iteration to ensure you cannot end up in an infinite loop.
njgheorghita
force-pushed
the
ethpm-bugfix
branch
from
293e8d9
to
3be068f
Compare
What was wrong?
w3.pm.get_all_packages()andw3.pm.get_all_release_ids()were wonky when dealing with large amounts of packages / releases. This can be seen here and here.The tests only covered cases with ~6 releases which worked fine.
Cute Animal Picture