Skip to content
This repository was archived by the owner on Oct 21, 2024. It is now read-only.

Valerian.roche/1.1.0+dd.3#1

Merged
valerian-roche merged 6 commits intomasterfrom
valerian.roche/1.1.0+dd.3
Jan 5, 2021
Merged

Valerian.roche/1.1.0+dd.3#1
valerian-roche merged 6 commits intomasterfrom
valerian.roche/1.1.0+dd.3

Conversation

@valerian-roche
Copy link

@valerian-roche valerian-roche commented Jan 4, 2021 •
edited
Loading

Currently there is no possibility to define a timeout (per call or globally) on consul to avoid stalling the thread in case of network issue or latency on consul side. This has already led to incidents

@valerian-roche valerian-roche force-pushed the valerian.roche/1.1.0+dd.3 branch from d94c2da to 6fbc06a Compare January 4, 2021 21:28
@valerian-roche valerian-roche changed the base branch from master to claudia/v1.1.0+dd.2 January 4, 2021 21:42
@valerian-roche valerian-roche changed the base branch from claudia/v1.1.0+dd.2 to master January 5, 2021 02:06
@valerian-roche valerian-roche marked this pull request as ready for review January 5, 2021 02:15
@wdauchy
Copy link

wdauchy commented Jan 5, 2021 •
edited
Loading

I recommend to have a look at https://github.com/criteo-forks/py-consul (https://pypi.org/project/py-consul/)
which is a maintained fork of python consul instead of creating another one.
it does include support for timeout https://github.com/criteo-forks/py-consul/blob/master/consul/aio.py#L14

@valerian-roche
Copy link
Author

valerian-roche commented Jan 5, 2021

I recommend to have a look at https://github.com/criteo-forks/py-consul (https://pypi.org/project/py-consul/)
which is a maintained fork of python consul instead of creating another one.
it does include support for timeout https://github.com/criteo-forks/py-consul/blob/master/consul/aio.py#L14

As discussed, definitely worth the migration once dogweb will be in python3 (as they dropped support for python2 that won't work for now). We'd also need to check if the "check_pid" this fork was created for is actually useful or not

@valerian-roche valerian-roche merged commit 104c8c4 into master Jan 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@platinummonkey platinummonkey platinummonkey approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@valerian-roche @wdauchy @platinummonkey @cablehead @claudiadadamo