0xPlaygrounds / subgrounds

An intuitive Python library for interfacing with subgraphs and GraphQL
http://docs.playgrounds.network
Apache License 2.0
50 stars 8 forks source link

Fix `add` method of `Selection` class #39

Closed jeanchambras closed 1 year ago

jeanchambras commented 1 year ago

Issue introduced with v1.5.1 (2023-05-15)

In the previous PR #17, modifications to the _subgrounds/pagination/preprocess.py_ file introduced the use of Selection.add at 2 locations, intended to conserve the outer Selection arguments while adding subselections. However, the add method failed to retain the arguments of the outer selection.

Affected code sections:

Impact: This issue results in the removal of all arguments that are not used in the pagination process. For example, the block (Block Height) argument, which allows selecting the block at which to return the data, is unintentionally removed. The queries are thus always run on the latest block available in the subgraph.

Fix: To address this issue, the following actions were taken:

This PR resolves the problem and ensures that the pagination process functions as intended, preserving the necessary arguments while adding subselections.

Recommandations:

0xMochan commented 1 year ago

Hey, somehow this completely slipped by radar! Thanks for the PR ❤️

0xMochan commented 1 year ago

I've reproduced with the aave-v2 subgraph from messari.

from subgrounds import Subgrounds
sg = Subgrounds()
aave_v2 = sg.load_subgraph("https://api.thegraph.com/subgraphs/name/messari/aave-v2-ethereum")

sg.query_df(aave_v2.Query.marketHourlySnapshots(first=10, block={"number": 20000000})

This code snippet, spits data out in current v1.6.0 while in this branch, it correctly outputs (matching GraphiQL):

GraphQLError: [{'message': 'Failed to decode `block.number` value: `subgraph Qmbte6tRTqFn9tf9LudDC9DLGnyKtqRqygXAUtj8FmKdye has only indexed up to block number 17823185 and data for block number 20000000 is therefore not yet available`'}]

Thank you for finding this issue and this PR @jeanchambras!

In the future, do try to create an issue with specific code-snippets with replication details as it makes it easier for me to verify the problem and the solution. This fix was simple enough (unfortuntely slipped from my previous bug fix), so it wasn't too big of a deal.

jeanchambras commented 1 year ago

Understood thx for the recommandation and the quick react! Following next development closely 👀