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

Adding timeout capability to api_call #27

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Adding timeout capability to api_call #27

merged 1 commit into from
Oct 19, 2020

Conversation

edvgui
Copy link

@edvgui edvgui commented Oct 16, 2020

Working on a project using this api, we recently faced the need to add a timeout feature to api_call. This allowed us to cancel some calls that wouldn't end (for reason not related to this api). This is working great for us so far, and we propose to merge it upstream.

The change made is simple, when calling api_call, an optional (no backward compatibility issue) field called timeout can be filled. Any positive integer will be translated as a timeout (in seconds) before which the call has to finish. Otherwise the request will be terminated and a TimeoutException (inheriting from ApiException) will be raised.

@chkp-yaelg
Copy link
Contributor

Thank you @edvgui ,
I will test this pull request during this week.

Can you please share a specific case where you are using it?
AFAIK it can happen when using our API commands not as expected or in case of a real issue. If it is the second case then you should contact support.

@edvgui
Copy link
Author

edvgui commented Oct 19, 2020

The issue that got us to implement this is actually not new with the checkpoint we are configuring: https://community.checkpoint.com/t5/API-CLI-Discussion-and-Samples/put-file-and-run-as-REST-API-script-keeps-showing-as-Task-in/td-p/10602

But in any case, even for new issue we might not know about yet, we would prefer to have a failure than and endless wait. The tool using the api is the Inmanta orchestrator, it might recover from a deployment failure, but not if it is stuck.

@chkp-yaelg chkp-yaelg merged commit 007984e into CheckPointSW:master Oct 19, 2020
@chkp-yaelg
Copy link
Contributor

Thank you @edvgui ,
You can find your fix in the new release v.1.1.2

@edvgui edvgui deleted the improvement/api-call-timeout branch October 19, 2020 08:16
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