Skip to content

fixed immediate timeout#283

Merged
fredbi merged 1 commit intogo-openapi:masterfrom
fredbi:fix/126-timeout
Dec 18, 2023
Merged

fixed immediate timeout#283
fredbi merged 1 commit intogo-openapi:masterfrom
fredbi:fix/126-timeout

Conversation

@fredbi
Copy link
Copy Markdown
Member

@fredbi fredbi commented Dec 13, 2023

fixed immediate timeout

This PR implements the proposal N°1 exposed in issue #126:

  • if there is a non-zero timeout set with the request, we
    add this timeout to the operation context

  • applicable contexts are:
    runtime.Context -> operation.Context -> context.WithTimeout(t>0)

  • the actual behavior is determined by the shortest deadline set for those
    contexts

  • added unit tests to assert that the different contexts are formed
    correctly:

    • an operation with no context inherits from the runtime context
    • an operation with no timeout set (by context or parameter) defaults to 30s (default timeout)
    • when several timeouts are set, the shortest wins
    • an operation with no timeout in context and a 0 timeout param waits
      for ever (and not with immediate timeout)

Signed-off-by: Frédéric BIDON fredbi@yahoo.com

@fredbi
Copy link
Copy Markdown
Member Author

fredbi commented Dec 13, 2023

TODO: add some unit tests -> Done

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (442694d) 82.27% compared to head (d337653) 82.74%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #283      +/-   ##
==========================================
+ Coverage   82.27%   82.74%   +0.47%     
==========================================
  Files          45       45              
  Lines        3447     3454       +7     
==========================================
+ Hits         2836     2858      +22     
+ Misses        499      489      -10     
+ Partials      112      107       -5     
Flag Coverage Δ
oldstable 82.74% <100.00%> (+0.47%) ⬆️
stable 82.74% <100.00%> (+0.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

* fixes go-openapi#126

This PR implements the proposal N°1 exposed in issue go-openapi#126:
* if there is a non-zero timeout set with the request, we
  add this timeout to the operation context
* applicable contexts are:
   runtime.Context -> operation.Context -> context.WithTimeout(t>0)
* the actual behavior is determined by the shortest deadline set on those
  contexts

* added unit tests to assert that the different contexts are formed
correctly:
  * an operation with no context inherits from the runtime context
  * an operation with no timeout set (by context or parameter) defaults to 30s (default timeout)
  * when several timeouts are set, the shortest wins
  * an operation with no timeout in context and a 0 timeout param waits
for ever (and not with immediate timeout)

Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
@fredbi fredbi marked this pull request as ready for review December 14, 2023 15:01
@fredbi fredbi merged commit 552b300 into go-openapi:master Dec 18, 2023
@fredbi fredbi deleted the fix/126-timeout branch December 26, 2023 16:01
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.

Client timeout of 0 causes immediate "context deadline exceeded"

2 participants