ImgProxy CDN #66

coreyja · 2024-06-10T16:26:52Z

I setup an instance of imgproxy and a BunnyCDN in front of it to help handle images for me

The idea is that the site can now request images from imgproxy and ask for them at specific sizes and dimensions, likely inside a picture HTML element so the browser can pick the size thats best

coreyja · 2024-06-10T16:28:06Z

server/src/http_server/pages/blog/md/html.rs

+        let adjusted_url = if let Some(imgproxy_url) = config.imgproxy_url.as_ref() {
+            format!(
+                "{}/unsafe/rs:auto:1000:0:false:false/plain/{}",
+                imgproxy_url,
+                urlencoding::encode(&self.url)
+            )
+        } else {
+            self.url
+        };


The issue I'm running into here that needs to be solved is that the self.url here is likely a relative path to the image, which isn't something I can send to imgproxy

So I need to get the URL of the page we are rendering in here to be able to move forward

coreyja · 2024-07-30T14:56:32Z

URL for imgproxy: https://imgproxy-cja.fly.dev/

Seif-Mamdouh · 2024-08-01T21:44:57Z

server/src/http_server/cmd.rs

    ];
+
+    if std::env::var("CRON_DISABLED").unwrap_or_else(|_| "false".to_string()) != "true" {


This code checks an environment variable to determine whether to enable ordisable cronjobs.

server/src/http_server/pages/blog/md/html.rs

Seif-Mamdouh · 2024-08-01T22:02:46Z

server/src/http_server/pages/blog/md/html.rs

-        _config: &AppConfig,
-        _context: &SyntaxHighlightingContext,
-    ) -> Result<Markup> {
+    fn into_html(self, config: &AppConfig, context: &SyntaxHighlightingContext) -> Result<Markup> {


function into_html take the
Markdown image syntax into HTML, constructing the image URL by combining the base URL, article path, and image filename.

Seif-Mamdouh · 2024-08-01T22:06:07Z

server/src/http_server/pages/blog/mod.rs

-        .ast
-        .into_html(&state.app, &state.markdown_to_html_context)
-    {
+    // creating some context for the article_path


context vat attempts to clone the existing markdown-to-html context and updates it with the current article's path

coreyja · 2024-08-02T13:52:48Z

.gitignore

@@ -17,3 +17,5 @@ models
 tmp_base_model

 server/src/local.rs
+
+CRON_DISABLED=true


Hehe wrong file for this 😉

This wants to be in your .envrc file (any likely is I feel like lol)

I put it there by accident lol

coreyja · 2024-08-02T13:53:37Z

tailwindcss

Ahh we don't want this binary file in the repo! It should also be in your ~/bin dir so I think we can leave it off here!

server/src/http_server/pages/blog/md/html.rs

coreyja · 2024-08-02T13:56:39Z

server/src/http_server/pages/blog/md/html.rs

+            .split('/')
+            .next()


Would be nice to not do any path parsing 'manually' 🤔

This is grabbing the stuff before the first / right? Whats after the slash lol?

In my local directory, I have a URL like this: http://localhost:3000/posts/BattlesnakeMinimax/Minimax%20in%20Battlesnake/. I only need the part of the URL before Minimax%20in%20Battlesnake for the image path. To achieve this, I split the URL and grab everything I need before Minimax%20in%20Battlesnake

server/src/http_server/pages/blog/md/html.rs

coreyja · 2024-08-02T13:59:09Z

server/src/http_server/pages/blog/md/html.rs

+            "{}/posts/{}/{}",
+            config.base_url.trim_end_matches('/'),
+            article_path,
+            self.url.trim_start_matches("../")


Would be nice to avoid this path parsing too!

Could we use some Rust Path structs/methods to like join the paths together? I feel like if we told it to join the paths and one has a ../ it will do the 'correct' thing and move up a directory there! 🤞

coreyja

Looking great!

Left some comments on the structs and then on the manual path parsing logic!

…get the full url here which requires piping context down farther

… related to SyntaxHighlightingContext

coreyja · 2024-08-04T01:02:14Z

server/src/http_server/pages/blog/md/html.rs

@@ -19,6 +21,7 @@ use crate::AppConfig;
 pub struct SyntaxHighlightingContext {
    pub(crate) theme: syntect::highlighting::Theme,
    pub(crate) syntax_set: syntect::parsing::SyntaxSet,
+    pub(crate) current_article_path: Option<String>,


We can remove this now that we have the new MarkdownRenderContext struct right?

Did this come back in a merge maybe?

We have the MarkdownRenderContext so I think we can/want to remove this path from this struct!

coreyja · 2024-08-04T02:00:46Z

server/src/http_server/pages/blog/md/html.rs

-            .unwrap_or("unknown");
-
-        let base_url = PathBuf::from(config.base_url.trim_end_matches('/'));
-        let image_path = PathBuf::from(self.url.trim_start_matches("./"));


./ means that we stay in the current path so this is actually fine to keep in!

The PathBuf::join method know hows to handle this and strips it our for us!

You can also call join and pass it a &str so you don't have to convert it to a PathBuf first!

coreyja · 2024-08-04T02:15:42Z

Hey @Seif-Mamdouh!

I pulled down the code to check it running and it run great for me without any changes! So don't think you broke it at the end lol

I was curious about a refactor and then kinda just did it 😆

Here is a link to the commit where I did most of the refactoring. I wrote a longer commit message so hopefully reading that can kinda show my thought processes since I didn't record doing this refactor.
93dd2d5 (#66)

And then also just highlighting these two code level comments I left on your changes before I refactored them: #66 (comment)

I think this is looking really good for getting the absolute URL on image now!
But one thing I think that got lost is using imgproxy so how about on Monday you give a shot to adding that back and better than the test version I had before!

So what we want to do is when we make the Image struct into html instead of using an 'old school' img tag lets us the newer picture tag which lets us do some more fancy things!

This is a good article on how to use this to serve different sized images to different sized devices! https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images

The idea is that we want to do that! But we only have the original sized images in our repo. So instead of linking to hard coded resized images, we want to link to imgproxy images so that it can do the resizing on the fly! And we will actually run imgproxy behind a CDN so we only have to generate each image size once!

Here are the imgproxy docs on how to construct a URL that has different options: https://docs.imgproxy.net/usage/processing#resize

…get the full url here which requires piping context down farther

… related to SyntaxHighlightingContext

posts/.DS_Store

server/src/http_server/cmd.rs

coreyja · 2024-08-08T17:05:20Z

server/src/http_server/pages/blog/md/html.rs

+        let small_url = generate_imgproxy_url(base_url, &img_src, 300, 300);
+        let medium_url = generate_imgproxy_url(base_url, &img_src, 600, 600);
+        let large_url = generate_imgproxy_url(base_url, &img_src, 1200, 1200);


For these can we maybe do height 0 so that imgproxy calculates the right height based on the width?

Since we are using the width for picking the right image source (which I like!) I think we might want to just use that and leave off the heights and let imgproxy figure those out!

When set to 0, imgproxy will calculate resulting height using the defined width and source aspect ratio.
From: https://docs.imgproxy.net/usage/processing#resize

…xy url path

…guess to format of the fly url but I think thats ok

# Via GitHub * main: Remove Neon Review App branches on PR close (#70) # Conflicts: # .github/workflows/fly_review.yml

coreyja commented Jun 10, 2024

View reviewed changes

Seif-Mamdouh reviewed Aug 1, 2024

View reviewed changes

server/src/http_server/pages/blog/md/html.rs Outdated Show resolved Hide resolved

Seif-Mamdouh reviewed Aug 1, 2024

View reviewed changes

coreyja commented Aug 2, 2024

View reviewed changes

server/src/http_server/pages/blog/md/html.rs Outdated Show resolved Hide resolved

coreyja commented Aug 2, 2024

View reviewed changes

server/src/http_server/pages/blog/md/html.rs Outdated Show resolved Hide resolved

coreyja commented Aug 2, 2024

View reviewed changes

coreyja and others added 7 commits August 2, 2024 17:15

Get imgproxy setup, but since I have local paths to images I need to …

5fffb81

…get the full url here which requires piping context down farther

fixed the the image url ref for imgproxy

0d6dfd5

fixing some lint issues

8faf7aa

fixing lint issues in cmd.rs

491cc88

added the MarkdownRenderContext struct and add changes to the imports…

d8dd4a5

… related to SyntaxHighlightingContext

used PathBuf for more robust path handling

38cbd74

fixed rebasing errors

75a78fb

Seif-Mamdouh force-pushed the ca/main/imgproxy branch from 2379087 to 75a78fb Compare August 2, 2024 22:22

coreyja commented Aug 4, 2024

View reviewed changes

coreyja and others added 6 commits August 4, 2024 14:19

Get imgproxy setup, but since I have local paths to images I need to …

ad596dc

…get the full url here which requires piping context down farther

fixed the the image url ref for imgproxy

8662c55

fixing some lint issues

ce72314

fixing lint issues in cmd.rs

6a1da05

added the MarkdownRenderContext struct and add changes to the imports…

7236662

… related to SyntaxHighlightingContext

used PathBuf for more robust path handling

887766d

Seif-Mamdouh had a problem deploying to review August 5, 2024 22:41 — with GitHub Actions Failure

removed /bin/bash from the repo

bf17972

Seif-Mamdouh had a problem deploying to review August 5, 2024 22:51 — with GitHub Actions Failure

Remove binary tailwindcss file and add it to .gitignore

7eddb71

Seif-Mamdouh had a problem deploying to review August 6, 2024 15:07 — with GitHub Actions Failure

deleted tailwindcss bin file

a559649

Seif-Mamdouh temporarily deployed to review August 6, 2024 15:23 — with GitHub Actions Inactive

Seif-Mamdouh marked this pull request as ready for review August 6, 2024 15:29

coreyja commented Aug 8, 2024

View reviewed changes

posts/.DS_Store Outdated Show resolved Hide resolved

coreyja commented Aug 8, 2024

View reviewed changes

server/src/http_server/cmd.rs Show resolved Hide resolved

coreyja commented Aug 8, 2024

View reviewed changes

Seif-Mamdouh temporarily deployed to review August 8, 2024 17:10 — with GitHub Actions Inactive

added .DS to gitignore and added the suggested refactor on the imgpro…

1a6f9b4

…xy url path

Seif-Mamdouh temporarily deployed to review August 8, 2024 18:54 — with GitHub Actions Inactive

Use the env var if set for the imgproxy base url, and cleanup some

56bb13a

coreyja temporarily deployed to review August 11, 2024 18:41 — with GitHub Actions Inactive

coreyja added 3 commits August 11, 2024 14:43

Use a relative url unless imgproxy is setup

6e3b579

More commented out lines

878fabf

Save markdown var so we don't have to call method multiple times

8fa5b9a

coreyja temporarily deployed to review August 11, 2024 18:48 — with GitHub Actions Inactive

coreyja added 2 commits August 11, 2024 14:57

Just set the base url correctly the first time. Does mean we have to …

af24faf

…guess to format of the fly url but I think thats ok

Merge branch 'main' into ca/main/imgproxy

e7456a5

# Via GitHub * main: Remove Neon Review App branches on PR close (#70) # Conflicts: # .github/workflows/fly_review.yml

coreyja temporarily deployed to review August 11, 2024 19:00 — with GitHub Actions Inactive

Use one img tag with a srcset

fe582e4

coreyja deployed to review August 11, 2024 19:18 — with GitHub Actions View deployment

coreyja mentioned this pull request Aug 11, 2024

Resize Images #33

Closed

coreyja linked an issue Aug 11, 2024 that may be closed by this pull request

Create appropriate sized images #28

Closed

coreyja merged commit ed4f4e2 into main Aug 11, 2024
6 checks passed

coreyja deleted the ca/main/imgproxy branch August 11, 2024 19:25

coreyja temporarily deployed to review August 11, 2024 19:25 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ImgProxy CDN #66

ImgProxy CDN #66

coreyja commented Jun 10, 2024

coreyja Jun 10, 2024

coreyja commented Jul 30, 2024

Seif-Mamdouh Aug 1, 2024

Seif-Mamdouh Aug 1, 2024 •

edited

Loading

Seif-Mamdouh Aug 1, 2024

coreyja Aug 2, 2024

Seif-Mamdouh Aug 2, 2024

coreyja Aug 2, 2024

coreyja Aug 2, 2024

Seif-Mamdouh Aug 2, 2024 •

edited

Loading

coreyja Aug 2, 2024

coreyja left a comment

coreyja Aug 4, 2024

coreyja Aug 8, 2024

coreyja Aug 4, 2024

coreyja Aug 4, 2024

coreyja commented Aug 4, 2024 •

edited

Loading

coreyja Aug 8, 2024

Seif-Mamdouh Aug 8, 2024

		];

		if std::env::var("CRON_DISABLED").unwrap_or_else(\|_\| "false".to_string()) != "true" {

ImgProxy CDN #66

ImgProxy CDN #66

Conversation

coreyja commented Jun 10, 2024

Choose a reason for hiding this comment

coreyja commented Jul 30, 2024

Choose a reason for hiding this comment

Seif-Mamdouh Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seif-Mamdouh Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coreyja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coreyja commented Aug 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seif-Mamdouh Aug 1, 2024 •

edited

Loading

Seif-Mamdouh Aug 2, 2024 •

edited

Loading

coreyja commented Aug 4, 2024 •

edited

Loading