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

make sysrepo connection timeout switchable #1099

Merged
merged 3 commits into from Apr 24, 2018

Conversation

frimpler
Copy link
Contributor

Hi,

please consider this little modification which makes it possible to disable the connection timeout in the sysrepo client library by compiler switch.

Background:
In low performance environments, sysrepo clients might be too busy to answer in time, if the connection then times out it starts that things get worse. For this reason we want to be able to switch it off (and deal with the consequences, if necessary)

Regards,
Frank

@rkrejci
Copy link
Contributor

rkrejci commented Apr 19, 2018

I'm fine with the idea, but could you integrate setting the variable into cmake processing? Ideally, I would connect it it somehow with REQUEST_TIMEOUT using some special value (zero or negative value).

@frimpler
Copy link
Contributor Author

Please check the update.

Thanks,
Frank

CMakeLists.txt Outdated
@@ -194,6 +194,10 @@ set(NOTIF_AGE_TIMEOUT 60 CACHE INTEGER
set(NOTIF_TIME_WINDOW 10 CACHE INTEGER
"Time window (in minutes) for notifications to be grouped into one data file (larger window produces larger data files).")

if(REQUEST_TIMEOUT EQUAL "0" AND NOT LONG_REQUEST_TIMEOUT EQUAL "0")
message(FATAL_ERROR "LONG_REQUEST_TIMEOUT must be 0 when REQUEST_TIMEOUT is 0")
Copy link
Contributor

Choose a reason for hiding this comment

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

in this case I would set LONG_REQUEST_TIMEOUT automatically (maybe with some message to get know what it does), on the other hand, I would like to have a test (and error) for the case REQUEST_TIMEOUT > LONG_REQUEST_TIMEOUT.

@michalvasko michalvasko merged commit 7690a27 into sysrepo:devel Apr 24, 2018
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

3 participants