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

DE-819 | bring back REQUEST_TIMEOUT as a class attribute #344

Closed
wants to merge 1 commit into from

Conversation

aMahanna
Copy link
Member

@aMahanna aMahanna commented Jun 5, 2024

[Optional PR]

Unfortunately this is no longer supported, but we can achieve something similar with this PR (see the added test cases)

Current ways that we support setting request_timeout:

# 1
client_no_timeout = ArangoClient(request_timeout=None)

# 2
client_no_timeout = ArangoClient(http_client=DefaultHTTPClient(request_timeout=None))

# 3
class NoTimeoutHTTPClient(DefaultHTTPClient):
    REQUEST_TIMEOUT = None

    def __init__(self, *args, **kwargs):
        super().__init__(request_timeout=self.REQUEST_TIMEOUT, *args, **kwargs)

client_no_timeout = ArangoClient(http_client=NoTimeoutHTTPClient())

Comment on lines +126 to +130
class NoTimeoutHTTPClient(DefaultHTTPClient):
REQUEST_TIMEOUT = None

def __init__(self, *args, **kwargs):
super().__init__(request_timeout=self.REQUEST_TIMEOUT, *args, **kwargs)
Copy link
Member Author

Choose a reason for hiding this comment

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

previously, the NoTimeoutHTTPClient with a class REQUEST_TIMEOUT would work without needing an __init__. This is PR basically introduces the ability to support REQUEST_TIMEOUT again, but with the exception of needing an __init__

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.95%. Comparing base (e44a3e8) to head (1bb4488).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #344   +/-   ##
=======================================
  Coverage   95.95%   95.95%           
=======================================
  Files          26       26           
  Lines        4277     4278    +1     
=======================================
+ Hits         4104     4105    +1     
  Misses        173      173           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aMahanna aMahanna closed this Jun 21, 2024
@aMahanna aMahanna deleted the DE-819 branch June 21, 2024 12:41
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