-
Notifications
You must be signed in to change notification settings - Fork 4
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
refactor: store collection volume changes #576
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ItsANameToo
changed the base branch from
feat/collections-overview
to
develop
December 20, 2023 09:55
what's left for this @crnkovic |
@ItsANameToo finished the code but have updated the PR description with more context on this implementation |
ItsANameToo
requested changes
Jan 17, 2024
9 tasks
samharperpittam
approved these changes
Jan 18, 2024
ItsANameToo
approved these changes
Jan 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
https://app.clickup.com/t/86dqrnjn8
So, for this PR, I went with the approach of storing daily volume changes of collections and then averaging that value over a 1d/7d/30d period. We can use that average for sorting on the collections page. In #548, Alfy went with the route of automatically taking 1d/7d/30d volume averages from Mnemonic (by making 3 parallel API requests) when we fetch volume. Both approaches have pros and cons, which I'll try to highlight here.
After giving it some thought, this may or may not be the best way to do this, depending on what we want. A while back, we removed scheduled updating of collection metadata (owners, volume, traits). Because of that and the way this PR works, I brought back the scheduled updating of collection volume -- just because we calculate average based on the volume data from our database. If we take an example number of 1000 collections in our database, this would automatically put us at 30k API requests a month to Mnemonic. Mnemonic segregates API requests by type, and free plan allows 50k requests a month for this type of request.
From what I see in the code, we don't really use Mnemonic a lot anymore. We use it to update: collection banner, owners, traits, volume and collection activity. All of these (other than activity) are ran on-demand when you open the collection page and not on schedule. Collection activity fetching is a type of its own and that endpoint has 50k requests/month limit as well and is of a different type than volume.
This gives us basically very little Mnemonic usage at the moment, so that we could go back to scheduled for volume and monitor API usage. The pro of this approach is that we'll always have the latest volume changes over 1d/7d/30d period, unlike in #548, where it consumes less API quota for Mnemonic, but we can get old data since the 1d/7d/30d volume is updated on-demand (when you open the collection page) and not on a schedule, therefore sorting would not really "work" if you don't update volume for a lot of collections.
Update: I have updated the code to handle situations where we don't have enough data in our database to calculate average weekly/monthly volume. Now, whenever a new collection is added to the database, we'll dispatch the jobs to fetch average volumes for a collection. This would basically be 3 requests per newly-added collection, but run only once for a new collection. Then, we'll reuse that data until we fetch the daily volume enough times to calculate averages.
Checklist