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

Batch retrieval of download counts with /api/download_counts #401

Merged

Conversation

HebaruSan
Copy link
Contributor

@HebaruSan HebaruSan commented Oct 13, 2021

Motivation

CKAN queries SpaceDock for download counts every day. Currently this happens via the /api/mod/<mod_id> route, which means that for each SpaceDock mod that CKAN knows about, we do:

  • One HTTPS request with a lot of extra output that isn't needed
  • One SQL request with a lot of data sent that isn't needed

This isn't a big problem today, but it is mildly wasteful of resources that could otherwise be available for users.

Changes

Now a new /api/download_counts route is available that takes multiple mod_id form parameters as input, then runs one SQL query that returns the download counts for all the mods without extraneous info and returns them as one JSON object.

To test locally:

curl -d 'mod_id=1&mod_id=2&mod_id=3' http://localhost:5080/api/download_counts

Once this is live, KSP-CKAN/NetKAN-Infra#228 will allow download counts to be queried with just one HTTPS and SQL request per day.

@HebaruSan HebaruSan added Type: Improvement Area: Backend Related to the Python code that runs inside gunicorn Priority: Low Status: Ready labels Oct 13, 2021
@HebaruSan HebaruSan requested a review from DasSkelett October 13, 2021 16:35
@HebaruSan HebaruSan force-pushed the feature/api-download-counts branch from 1e07cba to 4c09ae8 Compare October 23, 2021 16:18
@HebaruSan HebaruSan added the Scope: Trivial Simple changes that should be easy to develop and review label Nov 13, 2021
@DasSkelett
Copy link
Member

I tend to prefer putting objects of equal "type" in a list instead of a dict in JSON.
So basically like this:

{
  "download_counts": [
    {
      "id": 0,
      "downloads": 999
    },
    {
      "id": 1,
      "downloads": 666
    }
  ]
}

instead of

{
  "0": 999,
  "1": 666
}

This helps if you need to map the JSON to a language-native class/struct/..., because you can almost always easily specify that something is a list of objects of a certain type, while it might not be easy to model a dict with arbitrary keys.

For CKAN it doesn't really matter because we don't do that and and simply iterate over the results (which is very easy thanks to Python's dynamically typed nature). But it could be useful for other people who might consume the API.

@HebaruSan HebaruSan force-pushed the feature/api-download-counts branch from 4c09ae8 to 8fc4ecf Compare March 12, 2022 20:54
@HebaruSan
Copy link
Contributor Author

I think it would have been pretty reliable to deserialize that into a Dictionary<int, int>, but I do see some value in being able to represent a single "pair" as one object. Changed, will rewrite the NetKAN-Infra PR next.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Very nice, thank you!

@HebaruSan HebaruSan merged commit 082ed46 into KSP-SpaceDock:alpha Mar 12, 2022
@HebaruSan HebaruSan deleted the feature/api-download-counts branch March 12, 2022 21:09
@HebaruSan
Copy link
Contributor Author

Command to get all counts from alpha:

curl -d $(seq 1 100|head -c -1|sed -e 's/^/mod_id=/'|tr '\n' '&') https://alpha.spacedock.info/api/download_counts|jq

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Backend Related to the Python code that runs inside gunicorn Priority: Low Scope: Trivial Simple changes that should be easy to develop and review Status: Ready Type: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants