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

Feature/auto edge httpfile #780

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

knobby-tires
Copy link

Fixes #734

What this adds:

Automatic edge handling for the HTTPFile resource. The goal is to make sure that when an HTTP file is served from a path, the file resource is properly tracked as a dependency.

How it does that:

HTTPFileResAutoEdges struct: Handles dependecy tracking by keeping a list of related resources and checks if they exist.

AutoEdges() method: If the resource is serving from a file path, it creates an automatic edge to the corresponding file resource.
If the data field is used instead, no edges are created.

How I tested:

After setting things up I ran the following curl commands

curl http://localhost:8080/test1  # Check if /test1 works
curl http://localhost:8080/test2  # Check if /test2 works
curl http://localhost:8080/dir/index  # Check if specific file works

@purpleidea
Copy link
Owner

Thanks for the patch! A few quick nits to help you get through CI and the easy stuff:

  1. Missing an EOL in your example file.
  2. Make everything a single commit.
  3. Get rid of the go.mod changes.
  4. Comments should be full sentences (capitalized/ends with punctuation) or short one-liners usually that aren't capitalized.

@knobby-tires
Copy link
Author

Awesome, thanks for the fast reply. I will fix all of that tomorrow afternoon.

@knobby-tires knobby-tires force-pushed the feature/auto-edge-httpfile branch from 9a3b751 to bda0535 Compare January 21, 2025 16:14
@knobby-tires
Copy link
Author

@purpleidea I consolodated to one commit, fixed comments, removed changes to go.mod, added EOL to my example file, and made my comments more readable/professional.

@purpleidea
Copy link
Owner

(The race tests will likely fail, this is OK, but check out the other tests which should pass if there are no issues.)

@knobby-tires knobby-tires force-pushed the feature/auto-edge-httpfile branch from bda0535 to 80fc226 Compare January 21, 2025 16:52
@knobby-tires
Copy link
Author

It looks like I forgot to run gofmt on http.go, I fixed that and amended it to the commit.

@purpleidea
Copy link
Owner

Your mcl file comments need the same fixups and it will also fail without tab indentation. (There's also a trailing newline at the end if you want a nit.)

Adds automatic dependency edge generation between http:file resources and their
underlying file resources. This ensures file resources exist before they can be
served via HTTP. Supports both direct files and directory serving.
@knobby-tires knobby-tires force-pushed the feature/auto-edge-httpfile branch from 80fc226 to 7817d13 Compare January 21, 2025 21:28
@knobby-tires
Copy link
Author

Okay got it, thanks for bearing with me. Does this look good?

@purpleidea
Copy link
Owner

CI needs a slightly different commit message, eg:

resources: Add auto edge support between http_file and file resources

And you have one vet issue. I also noticed you're missing a newline at the end of a file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

automatic edge between http:file and file
2 participants