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

Provide new glob library with experiment #2341

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Conversation

DrJosh9000
Copy link
Contributor

@DrJosh9000 DrJosh9000 commented Aug 31, 2023

Why a new glob library?

Why write a new glob library?

  • I have doubts about the use of fastwalk (which is vendored in go-zglob).
  • I have doubts that go-zglob could be adapted to fix some of the problems above (it translates glob patterns into regexes under the hood, which makes partial matching for candidate directories difficult, and the code harder to read).
  • There are probably other glob libraries that would work, but wheee I'd already started

I realise I'm sneaking in a significant amount of unreviewed code through a new dependency. Please forgive me 🙏 I wanted to iterate quickly on design for the new library. At this point I believe it's at syntax and feature parity with go-zglob, has fewer bugs, and will provide a better experience.

But because of the risks, it comes under an experiment.

@DrJosh9000 DrJosh9000 force-pushed the pdp-1020-glob-better branch 3 times, most recently from ab0997b to 5b996ce Compare August 31, 2023 07:14
agent/artifact_uploader.go Outdated Show resolved Hide resolved
@DrJosh9000 DrJosh9000 force-pushed the pdp-1020-glob-better branch 10 times, most recently from 7ffbf95 to 9b781da Compare September 5, 2023 01:44
@DrJosh9000 DrJosh9000 changed the title WIP: A better glob? Provide new glob library with experiment Sep 5, 2023
@DrJosh9000 DrJosh9000 marked this pull request as ready for review September 5, 2023 01:45
The new library can be enabled with the `use-zzglob` experiment.
Checks out each plugin to an directory that that is namespaced by the agent name. Thus each agent worker will have an isolated copy of the plugin. This removes the need to lock the plugin checkout directories in a single agent process with spawned workers. However, if your plugin directory is shared between multiple agent processes *with the same agent name* , you may run into race conditions. This is just one reason we recommend you ensure agents have unique names.

***Status:*** Likely to be the default behaviour in the future.
**Status:** Likely to be the default behaviour in the future.
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏 I was going to fix this... when I removed the experiment 😅

@DrJosh9000 DrJosh9000 merged commit e216f2b into main Sep 5, 2023
1 check passed
@DrJosh9000 DrJosh9000 deleted the pdp-1020-glob-better branch September 5, 2023 03:40
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.

2 participants