Skip to content

Ensure a sensible timeout for pgsql commands#61433

Merged
Ch3LL merged 4 commits into
saltstack:masterfrom
edevil:acruz/pg_timeout
Nov 1, 2022
Merged

Ensure a sensible timeout for pgsql commands#61433
Ch3LL merged 4 commits into
saltstack:masterfrom
edevil:acruz/pg_timeout

Conversation

@edevil

@edevil edevil commented Jan 9, 2022

Copy link
Copy Markdown
Contributor

The psql command has no default timeout, so if the server is not
responding or if some lock cannot be obtained, we will be waiting
forever.

What does this PR do?

Introduce a default timeout of 60s for all psql commands.

Previous Behavior

Module would wait forever for commands to return.

New Behavior

Module will wait at most 60s for psql command to complete.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

The psql command has no default timeout, so if the server is not
responding or if some lock cannot be obtained, we will be waiting
forever.

Introduce a default timeout of 60s for all psql commands.
@edevil edevil requested a review from a team as a code owner January 9, 2022 10:22
@edevil edevil requested review from krionbsd and removed request for a team January 9, 2022 10:22
@welcome

welcome Bot commented Jan 9, 2022

Copy link
Copy Markdown

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@Ch3LL Ch3LL added the P1 Priority 1 label Sep 19, 2022

@dmurphy18 dmurphy18 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be made configurable with the default being 60 secs.
There should also be tests to check that the value is operational

Also needs a changelog entry (one liner describing the fix)

@dmurphy18 dmurphy18 added needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases Sulfur v3006.0 release code name and version labels Sep 22, 2022
@MKLeb

MKLeb commented Oct 10, 2022

Copy link
Copy Markdown
Contributor

@edevil Have you been able to look at implementing @dmurphy18 's suggestion?

@edevil

edevil commented Oct 10, 2022

Copy link
Copy Markdown
Contributor Author

Hey @MKLeb. Sorry but no, I've been a bit busy.

@twangboy twangboy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs a test and a changelog

@Ch3LL Ch3LL removed needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases needs-changelog labels Nov 1, 2022
@Ch3LL Ch3LL merged commit 637530a into saltstack:master Nov 1, 2022
@welcome

welcome Bot commented Nov 1, 2022

Copy link
Copy Markdown

Congratulations on your first PR being merged! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Priority 1 Sulfur v3006.0 release code name and version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants