Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discovery undisbursed endpoint for rewards #1642

Merged
merged 11 commits into from
Jul 14, 2021
Merged

Conversation

jowlee
Copy link
Contributor

@jowlee jowlee commented Jul 13, 2021

Description

Adds /v1/challenges/undisbursed endpoint for discovery

Tests

Wrote tests for the query

How will this change be monitored?

it will not be

Closes AUD-718

@jowlee jowlee requested a review from piazzatron July 13, 2021 14:30
@jowlee jowlee added the discovery-node Discovery Node (previously known as Discovery Provider) label Jul 13, 2021
Copy link
Contributor

@piazzatron piazzatron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, gonna simplify this since we don't need to roll up aggregates

ns = Namespace("challenges", description="Challenge related operations")

get_undisbursed_challenges_route_parser = reqparse.RequestParser()
get_undisbursed_challenges_route_parser.add_argument("limit", required=False, type=int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thoughts about if having offset would be useful as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we can add it, but I think cursor (keyset) base pagination might be more resilient?
For reference, this currently uses completed_blocknumber as the pseudo offset, but maybe we should add another field for offset - will do.
The things I'm worried about is if we add an offset and the challenges are being disbursed as we go, we could potentially skip challenges b/c of the offset.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally missed completed_blocknumber. makes sense

discovery-provider/src/api/v1/models/challenges.py Outdated Show resolved Hide resolved
@jowlee jowlee requested a review from piazzatron July 14, 2021 15:19
piazzatron
piazzatron previously approved these changes Jul 14, 2021
Copy link
Contributor

@piazzatron piazzatron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome - just small things. Hyped to get this merged

discovery-provider/src/api/v1/models/challenges.py Outdated Show resolved Hide resolved
discovery-provider/src/challenges/challenge.py Outdated Show resolved Hide resolved
discovery-provider/tests/conftest.py Outdated Show resolved Hide resolved
discovery-provider/tests/test_undisbursed_challeges.py Outdated Show resolved Hide resolved
@jowlee jowlee merged commit 624a8b8 into master Jul 14, 2021
@jowlee jowlee deleted the jowlee-disc-undisbursed branch July 14, 2021 20:08
dmanjunath added a commit that referenced this pull request Jul 15, 2021
Bugfix

Discovery undisbursed endpoint for rewards (#1642)

* Add undisbursed challenges endpoint

* Fix lint in discovery

* Add undisbursed endpoint route

* Fix limit and endpoint name

* Addressed comments

* Fix lint

* Rebase changes

* Update discovery test around fixture

* Pass through arg

* Address comments

* Remove unused imports lint

Support rewards challenge fields (#1636)

* Support UserEvents in discovery indexing

* Support referrer and is_mobile_user in libs

* Add events to tasks metadata

* Address comments

* Fix misc type issues

* Remove unused imports

* Fix get_attestation type bug

* Fix lint

* Update down revision

Debugging

Debugging

Circle config update

config

Debug

Debug

Debug

Consolidate some logic

Consolidate some logic

Final cleanup + polish
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discovery-node Discovery Node (previously known as Discovery Provider)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants