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

Nits and stuff #3514

Merged
merged 7 commits into from
Aug 7, 2023
Merged

Conversation

webbnh
Copy link
Member

@webbnh webbnh commented Aug 4, 2023

This PR consists of a bunch of tiny fixes of minimal consequence which I've gradually accumulated over the last month. They were things which I ran into in the course of making other changes, but, since they had nothing to do with the changes I was making, I kept them on a side branch. At this point, there are enough of them to warrant putting up a review.

  • Remove unused definitions from the Server container build and from load_keycloak.sh
  • Correct a docstring
  • Correct the import of the urllib3 package and add a requirement for it: the package is apparently included inside the requests package, and we were referencing it there instead of declaring our own dependency on it.
  • Add defaults to the deploy-dependencies script: this script is normally invoked by a higher-level script which provides these definitions; if a user wants to run this script independently, then s/he has to "just know" to provide these definitions in the environment; this change provides reasonable default values if the variables are not already defined.
  • Try to report errors in the containerized Agent command invocation. The containerized Agent container images are intended to be invoked with a shell command (typically a Pbench Agent script) to run. If this is omitted, or if the container invocation command line is formed incorrectly with command options positioned after the image spec, then the exec call inside the script which is run as the container entrypoint will fail, and this results in a cryptic response (if any) for the user. This PR modifies the entry point script script to try to handle these cases better for the user.

@webbnh webbnh added Agent Server packaging Issues related to software packaging Containerization Of and relating to the process of setting up and maintaining container images labels Aug 4, 2023
@webbnh webbnh added this to the v0.73 milestone Aug 4, 2023
@webbnh webbnh self-assigned this Aug 4, 2023
@webbnh webbnh marked this pull request as ready for review August 4, 2023 20:08
@webbnh webbnh requested review from dbutenhof and ndokos August 4, 2023 20:09
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

There's a shell linter? And you're running it? Weird. 🍤

@webbnh
Copy link
Member Author

webbnh commented Aug 4, 2023

There's a shell linter? And you're running it? Weird. 🍤

Yeah, there is. Yeah, I am. And, yeah, it makes life "interesting". I don't necessarily agree with everything it says, but there are enough pitfalls in shell programming that I do appreciate having the heads-up when I'm doing something questionable (or when there is a better/alternate way to do it that I wasn't aware of).

@webbnh webbnh merged commit 647e9f2 into distributed-system-analysis:main Aug 7, 2023
@webbnh webbnh deleted the nits_and_stuff branch August 7, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent Containerization Of and relating to the process of setting up and maintaining container images packaging Issues related to software packaging Server
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants