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

Honor FOREMAN_REX_WORKDIR #16

Merged
merged 2 commits into from
Sep 21, 2023
Merged

Conversation

adamruzicka
Copy link
Contributor

Alternative to #15
Includes #13
Companion to theforeman/foreman-packaging#9817

This replaces the (as of Go 1.17) deprecated ioutil.TempFile with
os.CreateTemp. When calling os.CreateTemp with an empty dir the default
directory for temporary files is used. On UNIX that's $TMPDIR if
non-empty or /tmp.
@ezr-ondrej
Copy link
Member

@adamruzicka do you have someone to review? Ping me if you'd need me to :)

src/main.go Outdated
if err := s.Serve(l); err != nil {
log.Fatal(err)
}
}

func determineWorkdir() string {
workdir, workdirP := os.LookupEnv("FOREMAN_REX_WORKDIR")
Copy link
Member

Choose a reason for hiding this comment

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

I'd consider FOREMAN_YGG_WORKER_WORKDIR or FOREMAN_YGG_WORKDIR as I think that would follow the convention that is being targeted for environment variable declaration of workers. e.g. https://github.com/theforeman/foreman-operations-collection/blob/develop/roles/cloud_connector/templates/foreman_rh_cloud.toml.j2#L3-L8

Although maybe going the route you have here with a change in the future to a naming scheme that is yggdrasil-worker-foreman-rex would better reflect what it is and then this would be in line with that already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming is hard, changed to the first option you proposed

@adamruzicka adamruzicka marked this pull request as ready for review September 20, 2023 08:29
@adamruzicka adamruzicka merged commit 618d9ba into theforeman:main Sep 21, 2023
5 checks passed
@adamruzicka adamruzicka deleted the working-dir2 branch September 21, 2023 08:35
This was referenced Sep 21, 2023
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.

None yet

4 participants