Skip to content

Add md5 hash query param to images to bust the cache when we update one inplace #3746

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 25, 2024
Merged

Add md5 hash query param to images to bust the cache when we update one inplace #3746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
886e8f9
Add md5 hash suffix at end of image names to bust the cache when we u…
nathan-contino Jun 25, 2024
f1e1ffb
Fix build with apparent image file path
nathan-contino Jun 25, 2024
326911b
Should have just tried a query param from the beginning
nathan-contino Jun 25, 2024
4d4a980
Update scripts/create_auto_ninjabuild.py
nathan-contino Jun 25, 2024
e617a82
Add to top-level adoc parsing file in case anyone ever dumps an image…
nathan-contino Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions documentation/asciidoc/services/connect/use.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ image::images/screen-sharing-end.png[width="80%"]

To turn off screen sharing, click the Connect system tray icon and unselect **Allow screen sharing**. Your Raspberry Pi remains signed into Connect, but you won't be able to create a screen sharing session from the Connect dashboard.

image:images/screen-sharing-disabled-desktop.png[width="80%"]
image::images/screen-sharing-disabled-desktop.png[width="80%"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These work fine as inline images, but I messed up the source asciidoc here when I wrote the page. Turned out to be some nice 'test cases' because they were the only images not detected by my regex!

Copy link
Contributor

Choose a reason for hiding this comment

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

Or you could fix your regex so that it works with both one or two colons? ::?


Alternatively, you can disable screen sharing with the following command:

Expand Down Expand Up @@ -168,7 +168,7 @@ image::images/remote-shell-end.png[width="80%"]

To turn off remote shell access, click the Connect system tray icon and unselect **Allow remote shell**. Your Raspberry Pi remains signed into Connect, but you won't be able to create a remote shell session from the Connect dashboard.

image:images/remote-shell-disabled-desktop.png[width="80%"]
image::images/remote-shell-disabled-desktop.png[width="80%"]

Alternatively, you can disable remote shell access with the following command:

Expand Down
7 changes: 6 additions & 1 deletion scripts/create_build_adoc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ def check_no_markdown(filename):
if re.search(r'(\[.+?\]\(.+?\))', asciidoc):
raise Exception("{} contains a Markdown-style link (i.e. '[title](url)' rather than 'url[title]')".format(filename))


if __name__ == "__main__":
index_json = sys.argv[1]
config_yaml = sys.argv[2]
Expand Down Expand Up @@ -72,6 +71,12 @@ def check_no_markdown(filename):
m = re.match(r'^(include::)(.+)(\[\]\n?)$', line)
if m:
line = m.group(1) + os.path.join('{includedir}/{parentdir}', m.group(2)) + m.group(3)
# find all image references, append md5 hash at end to bust the cache if we change the image
m = re.match(r'^(image::)(.+)(\[(.+)]\n?)$', line)
if m:
directory = os.path.dirname(os.path.abspath(src_adoc))
image_hash = hashlib.md5(open(os.path.join(directory, m.group(2)),'rb').read()).hexdigest()
line = m.group(1) + m.group(2) + '?hash=' + image_hash + m.group(3) + "\n"
new_contents += line

with open(build_adoc, 'w') as out_fh:
Expand Down
8 changes: 8 additions & 0 deletions scripts/create_build_adoc_include.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import re
import yaml
import hashlib


def check_no_markdown(filename):
Expand Down Expand Up @@ -48,6 +49,13 @@ def check_no_markdown(filename):
seen_header = True
if github_edit is not None:
line += edit_text + "\n\n"
else:
# find all image references, append md5 hash at end to bust the cache if we change the image
m = re.match(r'^(image::)(.+)(\[(.+)]\n?)$', line)
Copy link
Contributor

Choose a reason for hiding this comment

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

@nathan-contino Do we need to use non-greedy matching here? E.g. .+? instead of .+? Since it's just one line, it's pretty low risk, but I always get nervous about this stuff. But also not sure if that would change the functionality at all...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's valid syntax to put things on a line after an image other than the image itself, and most images should have the square brackets. So we can probably get away without worrying. I couldn't find a less-greedy approach that worked with all images, since names vary a lot!

Copy link
Contributor

@lurch lurch Jun 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Does the square-brackets-part at the end need to be made an optional match?
EDIT: Also, looks like you probably don't need the capture-group (round brackets) inside the literal square brackets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually plan to add alt text to every one of the images in our docs, so... technically no. But it might be safer to do so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point that we can likely improve this regex. However, I've tested this one (to verify that it works for all images in our repo) and need it for another PR, so I'm just going to roll ahead with this version right now. I will circle back and try to improve it at a later date!

if m:
directory = os.path.dirname(os.path.abspath(src_adoc))
image_hash = hashlib.md5(open(os.path.join(directory, m.group(2)),'rb').read()).hexdigest()
line = m.group(1) + m.group(2) + '?hash=' + image_hash + m.group(3) + "\n"
new_contents += line

with open(build_adoc, 'w') as out_fh:
Expand Down