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

docs: Add example job that shuts down proxy containers on completion. #436

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

hessjcg
Copy link
Collaborator

@hessjcg hessjcg commented Sep 14, 2023

Demonstrate how to configure a job so that job containers exit successfully when the job completes.

Related to #361

command:
- sh
- -c
- "psql --host=$DB_HOST --port=$DB_PORT --username=$DB_USER '--command=select 1' --echo-queries --dbname=$DB_NAME\nfor url in $CSQL_QUIT_URLS ; do \n wget --post-data '' $url \ndone\n"
Copy link
Member

Choose a reason for hiding this comment

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

Does the busybox image have curl? I think that might be more clear than wget.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The busybox image has wget but not curl.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok. Well either way, we can change this to a get request once the next version of the Proxy is released yeah?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Member

Choose a reason for hiding this comment

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

Can we clean up the whitespace here? Is there any way to make this more readable? If we end up putting this in the docs one day, the horizontal scroll will be brutal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. This is now fixed.

@hessjcg hessjcg requested a review from enocom September 19, 2023 15:28
@hessjcg hessjcg force-pushed the gh-361-job-docs branch 2 times, most recently from 0985fe0 to 04427d4 Compare September 20, 2023 17:07
# This demonstrates how to configure a batch job so that it shuts down
# the proxy containers when it has finished processing.
#
# The operator will set an environment variable called CSQL_QUIT_URLS
Copy link
Member

Choose a reason for hiding this comment

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

I missed this in review, but shouldn't this be CSQL_PROXY_QUIT_URLS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, glad you caught this. I opened #454 with the fix. I'll update this PR too.

# so the value of CSQL_QUIT_URLS will be something like
# "http://localhost:9091/quitquitquit"
#
# The main job container should send a POST request each URL when the job
Copy link
Member

Choose a reason for hiding this comment

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

POST or GET? GET is so much more convenient.

Copy link
Member

Choose a reason for hiding this comment

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

Also "request TO each URL"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

@hessjcg hessjcg requested a review from enocom October 3, 2023 20:55
@hessjcg hessjcg enabled auto-merge (squash) October 9, 2023 17:48
@hessjcg hessjcg merged commit ac4c4b5 into main Oct 9, 2023
8 checks passed
@hessjcg hessjcg deleted the gh-361-job-docs branch October 9, 2023 17:55
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