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

config: expose http timeout in config schema #8722

Merged
merged 2 commits into from
Jan 5, 2023

Conversation

dtrifiro
Copy link
Contributor

@dtrifiro dtrifiro commented Dec 21, 2022

  • read_timeout maps to aiohttp's sock_read timeout (read timeout while waiting for a new portion of data from the peer)
  • connect_timeout applies to aiohttp's connect, sock_connect timeouts

For more information:
https://docs.aiohttp.org/en/stable/client_reference.html?highlight=clienttimeout#clienttimeout

@codecov
Copy link

codecov bot commented Dec 21, 2022

Codecov Report

Base: 93.52% // Head: 93.51% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (68b2430) compared to base (f386f7c).
Patch has no changes to coverable lines.

❗ Current head 68b2430 differs from pull request most recent head b817717. Consider uploading reports for the commit b817717 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8722      +/-   ##
==========================================
- Coverage   93.52%   93.51%   -0.01%     
==========================================
  Files         457      457              
  Lines       36229    36195      -34     
  Branches     5252     5242      -10     
==========================================
- Hits        33882    33848      -34     
+ Misses       1840     1839       -1     
- Partials      507      508       +1     
Impacted Files Coverage Δ
tests/unit/repo/experiments/conftest.py 90.62% <0.00%> (-4.69%) ⬇️
dvc/repo/experiments/queue/celery.py 85.37% <0.00%> (-2.69%) ⬇️
dvc/repo/experiments/remove.py 95.23% <0.00%> (-1.74%) ⬇️
dvc/repo/experiments/__init__.py 87.00% <0.00%> (-1.09%) ⬇️
dvc/repo/experiments/queue/base.py 85.95% <0.00%> (-0.14%) ⬇️
tests/unit/repo/experiments/queue/test_celery.py 96.39% <0.00%> (-0.04%) ⬇️
tests/func/experiments/test_experiments.py 99.72% <0.00%> (-0.01%) ⬇️
dvc/stage/run.py 94.04% <0.00%> (ø)
dvc/commands/queue/kill.py 100.00% <0.00%> (ø)
tests/unit/command/test_queue.py 100.00% <0.00%> (ø)
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dtrifiro
Copy link
Contributor Author

dtrifiro commented Dec 21, 2022

Related: iterative/dvc-http#25

@dtrifiro dtrifiro marked this pull request as ready for review December 23, 2022 17:55
dvc/config_schema.py Outdated Show resolved Hide resolved
Copy link
Member

@skshetry skshetry left a comment

Choose a reason for hiding this comment

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

LGTM (the test that is failing also should not be here).

tests/unit/fs/test_http.py Outdated Show resolved Hide resolved
- `read_timeout` timeout for reading new data portions from the peer
- `connect_timeout` timeout for establishing new connections

For more information about individual timeouts meanings:

- `read_timeout` maps to `aiohttp`'s `sock_read` timeout
- `connect_timeout` applies to `aiohttp`'s `connect`, `sock_connect`
  timeouts

https://docs.aiohttp.org/en/stable/client_reference.html?highlight=clienttimeout#clienttimeout
@dtrifiro dtrifiro marked this pull request as ready for review January 5, 2023 19:59
@dtrifiro dtrifiro enabled auto-merge (rebase) January 5, 2023 20:01
@dtrifiro dtrifiro merged commit 33de761 into iterative:main Jan 5, 2023
@dtrifiro dtrifiro deleted the expose-http-timeout branch January 5, 2023 20:17
dtrifiro added a commit to iterative/dvc.org that referenced this pull request Jan 9, 2023
dtrifiro added a commit to iterative/dvc.org that referenced this pull request Jan 9, 2023
dtrifiro added a commit to iterative/dvc.org that referenced this pull request Jan 23, 2023
dberenbaum pushed a commit to iterative/dvc.org that referenced this pull request Jan 23, 2023
… add`) (#4223)

* remote modify: remove duplicate config options for s3

* remote modify: add timeout config options for http remote

Related: iterative/dvc#8722

* Update content/docs/command-reference/remote/modify.md

* Update content/docs/command-reference/remote/modify.md

* Revert "Update content/docs/command-reference/remote/modify.md"

This reverts commit 5cd4c2b.

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.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.

2 participants