-
Notifications
You must be signed in to change notification settings - Fork 18
feat: Keep the binary in the plugin instead of build dir #135
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
base: main
Are you sure you want to change the base?
Conversation
toote
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the 2 comments, I would also suggest allowing it to be configured in case users want to change that to something else with an option (maybe something like download-folder) that defaults to the value of BUILDKITE_PLUGINS_PATH. What do you think?
|
If you like, we could make it configurable with a env var that would take priority if set like |
Yeah, I suggested something like that but recommended calling it |
README.md
Outdated
| ``` | ||
|
|
||
| ## Go Binary | ||
| By default, the Go binary is downloaded to `BUILDKITE_PLUGINS_PATH`. If you want to download the binary to a different location, set the `BUILDKITE_PLUGIN_MONOREPO_DIFF_DOWNLOAD_FOLDER` environment variable to your desired directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable will get set if you add the download-folder option to the plugin itself (which will also provide the best place to document the functionality)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're suggesting that we should make download-folder an option in the pipeline.yml usage of the plugin. Maybe there's something I don't understand, but wouldn't that require us to parse the yaml in Bash, which is what the Go binary is actually built for?
The reason I went with an env var is because those would be accessible in Bash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Options passed to the plugin are also available as bash variables by the agent. Which means that a download-folder option for this plugin will be available as BUILDKITE_PLUGIN_MONOREPO_DIFF_DOWNLOAD_FOLDER to its hooks... so it would not require any further changes to the code you created, just the documentation and the plugin.yml file :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's great!
499eb14 to
a10eb27
Compare
|
I think I've addressed your feedback here. Is there anything else I should change? |
toote
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor issue with the linter and I think that this can be approved and merged :)
| steps: | ||
| - label: "Triggering pipelines" | ||
| plugins: | ||
| - monorepo-diff#v1.5.2: | ||
| download_folder: "/var/buildkite-agent" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should correct the linting issues:
| steps: | |
| - label: "Triggering pipelines" | |
| plugins: | |
| - monorepo-diff#v1.5.2: | |
| download_folder: "/var/buildkite-agent" | |
| steps: | |
| - label: "Triggering pipelines" | |
| plugins: | |
| - monorepo-diff#v1.6.1: | |
| download_folder: "/var/buildkite-agent" | |
| watch: | |
| - path: "bar-service/" | |
| config: | |
| key: echo-step | |
| command: "echo deploy-bar" |
This fixes Issue #76
Follow up to #127
I didn't have a full understanding of where the binary was getting saved, so these changes are required to actually keep from having to download the binary every time.
Prior to this change, the binary was downloaded to the builds directory,
/var/lib/buildkite-agent/builds/...on my machine, which gets its changes cleaned up between every run. Instead we want to save the binary in the plugins directory,/etc/buildkite-agent/plugins/...on my machine, so that it persists between runs. I followed the same pattern as the pr commenter buildkite plugin.