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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set OCNative request timeout to 60s #3549

Merged
merged 3 commits into from Jun 5, 2023

Conversation

hemslo
Copy link
Contributor

@hemslo hemslo commented Jun 5, 2023

OCNative request may hang indefinitely without timeout, this change adds a default timeout 60s.

APPSRE-7677

timeout-settings.md

example request_timeout.py

馃 Generated by Copilot at a1ca233

Improved testing and timeout handling for the oc module. Renamed the oc fixture and parameter in test_utils_oc.py to avoid confusion with the native client. Added tests for the OCNative class and its methods. Added a 60-second timeout for native client requests in oc.py.

馃 Generated by Copilot at a1ca233

  • Add a constant REQUEST_TIMEOUT to the oc module and use it to set the timeout for the native client requests (link, link, link, link, link)
  • Rename the oc fixture to oc_cli and annotate it with the OCCli type in test_utils_oc.py (link, link)
  • Rename and annotate the oc parameter to oc_cli in the test functions that use the oc_cli fixture (link, link, link, link, link)
  • Import the OCNative class from the oc module in test_utils_oc.py (link)
  • Add a new fixture oc_native that returns an OCNative instance with mocked API resources and client in test_utils_oc.py (link)
  • Add four new test functions to test the get, get_items, and get_all methods of the OCNative class using the oc_native fixture (link)

Signed-off-by: Di Wang <dwan@redhat.com>
Signed-off-by: Di Wang <dwan@redhat.com>
reconcile/utils/oc.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kwilczynski kwilczynski left a comment

Choose a reason for hiding this comment

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

@hemslo, looks good!

Thank you for adding this!

@kwilczynski
Copy link
Contributor

@hemslo, good to go after we fix the argument name.

Signed-off-by: Di Wang <dwan@redhat.com>
@hemslo hemslo merged commit e65e0c2 into app-sre:master Jun 5, 2023
1 check passed
@hemslo hemslo deleted the ocnative-request-timeout branch June 5, 2023 04:40
bkez322 pushed a commit to bkez322/qontract-reconcile that referenced this pull request Jul 13, 2023
Set OCNative request timeout to 60s

Signed-off-by: Di Wang <dwan@redhat.com>
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

2 participants