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

Improve upload-image artifact discoverability #2461

Closed
jprochazk opened this issue Jun 16, 2023 · 0 comments · Fixed by #3477
Closed

Improve upload-image artifact discoverability #2461

jprochazk opened this issue Jun 16, 2023 · 0 comments · Fixed by #3477
Labels
🧑‍💻 dev experience developer experience (excluding CI) 🏎️ Quick Issue Can be fixed in a few hours or less

Comments

@jprochazk
Copy link
Member

jprochazk commented Jun 16, 2023

scripts/upload_image.py first generates various sizes of the image, uploads all of them, and finally produces a snippet like:

<picture>
  <source media="(max-width: 1024px)" srcset="https://static.rerun.io/...>
  <!-- a few different sizes -->
  <img src="https://static.rerun.io/... original image.png">
</picture>

All the images are hashed separately after they've been resized, and they're also all uploaded separately into a flat directory. This is bad for searchability and when you have an image like https://static.rerun.io/<content-hash>_image.png, it isn't immediately clear if different resolutions for it are available.

We could instead calculate the content hash only for the original image, then create a directory like <original-image-content-hash>_<original-image-name>/, and then upload all the different resolutions (+ the original) into this directory. We'll continue to generate the pyramid in the same way.

@jprochazk jprochazk added the 🧑‍💻 dev experience developer experience (excluding CI) label Jun 16, 2023
@emilk emilk added the 🏎️ Quick Issue Can be fixed in a few hours or less label Jun 27, 2023
@jleibs jleibs changed the title Improve image discoverability Improve upload-image artifact discoverability Aug 7, 2023
This was referenced Sep 25, 2023
jprochazk added a commit that referenced this issue Sep 26, 2023
### What

* Fixes #2461

The new link format is
`https://static.rerun.io/{original_image_name_minus_extension}/{content_hash}/{width}.{ext}`

For example,
`https://static.rerun.io/ae4b8970edba8480431cb71e57b8cddd9e1769c7_clock_480w.png`
is now
`https://static.rerun.io/clock/ae4b8970edba8480431cb71e57b8cddd9e1769c7/480w.png`.

What makes this more discoverable:
- For machines, it's the fact that the hash is the same for all sizes.
This means given an image URL, you can retrieve all the other sizes.
This is important for #3445.
- For humans, the top-level directory is the name of the image, which
makes it _much_ easier to browse through GCS looking for that image. You
can compare the `updated at` date of each hash to find the latest one.

`upload_image.py` has been updated to use the new format. We're nowhere
near the point of using too much space on GCS, so all the old links are
preserved.

This PR also fixes some other issues:
- `thumbnails.py` did not handle updating `thumbnail_dimensions`
correctly if the property was already present in the frontmatter
- `upload_image.py` would fail if the user does not have `delete`
permission when uploading the same image twice, because it would attempt
to overwrite the existing image. if the content hash does not change,
then we do not need to upload the image again, so we just skip uploading
to GCS if the object already exists.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3477) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3477)
- [Docs
preview](https://rerun.io/preview/67a25d43432ddf6245f04ca52806fd976a4dac4e/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/67a25d43432ddf6245f04ca52806fd976a4dac4e/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) 🏎️ Quick Issue Can be fixed in a few hours or less
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants