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

✨ generate csv and zip file server side #3613

Merged
merged 2 commits into from
Oct 10, 2024
Merged

✨ generate csv and zip file server side #3613

merged 2 commits into from
Oct 10, 2024

Conversation

danyx23
Copy link
Contributor

@danyx23 danyx23 commented May 16, 2024

This PR adds functionality to generate a CSV file on the server in a CF worker for the data of any chart. It also allows downloading a metadata.json file, a readme.md and a zip file of all three of these things.

This is in preparation of surfacing these things in the download UI of grapher in the upcoming cycle 2024.6 (#4015). This PR does not make any use of the new CF functions endpoints yet and the download UI is not yet changed.

This PR implements #3648

Testing

To test this, try http get requests against /grapher/SLUG.zip, /grapher/SLUG.csv, /grapher/SLUG.metadata.json at localhost:8788, or at the staging server linked below.

There is also an observable notebook that lets you browse the generated readme files in an easier way: https://observablehq.com/d/d410e9b2d2b7c330

@owidbot
Copy link
Contributor

owidbot commented May 16, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-server-side-csv

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2024-07-31 07:59:25 UTC
Execution time: 1.22 seconds

Copy link

This PR has had no activity within the last two weeks. It is considered stale and will be closed in 3 days if no further activity is detected.

@danyx23 danyx23 force-pushed the server-side-csv branch 2 times, most recently from 1f9008e to 4549ceb Compare August 12, 2024 10:42
@danyx23 danyx23 changed the base branch from master to multiembedder-use-config-api August 26, 2024 20:46
@danyx23 danyx23 force-pushed the multiembedder-use-config-api branch from 4f6ed11 to 8e9dbef Compare August 28, 2024 15:50
@danyx23 danyx23 changed the base branch from graphite-base/3613 to master September 10, 2024 12:01
@danyx23 danyx23 force-pushed the server-side-csv branch 3 times, most recently from c1fdfd7 to d2b893f Compare September 16, 2024 12:34
@danyx23 danyx23 force-pushed the server-side-csv branch 2 times, most recently from 323f690 to e43bcd4 Compare October 4, 2024 20:50
@danyx23 danyx23 changed the title 🔬 experiment to create CSV in CF function ✨ generate csv and zip file server side Oct 4, 2024
@danyx23 danyx23 force-pushed the server-side-csv branch 2 times, most recently from 06b7fa9 to 396e567 Compare October 5, 2024 15:11
@danyx23 danyx23 marked this pull request as ready for review October 5, 2024 15:12
@danyx23 danyx23 linked an issue Oct 5, 2024 that may be closed by this pull request
15 tasks
@marcelgerber
Copy link
Member

marcelgerber commented Oct 10, 2024

There are some UTF-8 issues with the generated README.
Looking into those, nothing to do there for you for now.

CleanShot 2024-10-10 at 11 35 53@2x

EDIT: Fixed in #4051.

Copy link
Member

@marcelgerber marcelgerber left a comment

Choose a reason for hiding this comment

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

Nice!

I proposed some enhancements in #4051, so please have a look at that one.
Otherwise, this looks really good!

One thing I'm wondering is if in the metadata and README files, we want to note down the date of the download? I think that would be a nice idea.

Copy link
Member

Choose a reason for hiding this comment

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

Another note for future-us:
We should extract some of this functionality out of grapherRenderer, and split it up into some separate files for

  • parsing (image) options
  • initializing grapher
  • generating svg/png
  • readme/metadata file generation
  • csv file generation

Copy link
Contributor Author

danyx23 commented Oct 10, 2024

Merge activity

  • Oct 10, 12:53 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Oct 10, 12:54 PM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 10, 12:55 PM EDT: A user merged this pull request with Graphite.

@danyx23 danyx23 merged commit 5744b4b into master Oct 10, 2024
13 of 15 checks passed
@danyx23 danyx23 deleted the server-side-csv branch October 10, 2024 16:55
danyx23 added a commit that referenced this pull request Oct 10, 2024
Enhancements on top of #3613, see the commit messages for details.

The biggest change is replacing `jszip` with [`littlezipper`](https://www.npmjs.com/package/littlezipper), which (mostly) uses the browser-inbuilt (and Workers-inbuilt!) `CompressionStream` API for zip generation, which gives us fast, native compression.
It's also a good option if we ever should want to create a zip file directly in the browser - it uses `CompressionStream` if available ( = most modern browsers), or otherwise creates an uncompressed zip file as a fallback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable server-side csv download for all charts
4 participants