pytest-api test: fix ping server step #997
Conversation
|
Work on |
|
Hi @odulcy-mindee I think both PR's will not solve the under issue mentioned by users it is only a fix for CI 😅 |
|
@felixdittrich92 Yeah, I just discovered that haha |
|
A deprecated decorator from There has been a new release on |
Codecov Report
@@ Coverage Diff @@
## main #997 +/- ##
=======================================
Coverage 94.85% 94.85%
=======================================
Files 134 134
Lines 5558 5558
=======================================
Hits 5272 5272
Misses 286 286
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. |
|
@odulcy-mindee I think to solve this issue we should add tests for different envs (Linux, Windows, Mac) wdyt ? I have had some trouble to solve it in fact that i was not able to reproduce the issue :) |
frgfm
added
ext: api
|
Hey everyone 👋 Here are my humble thoughts about this:
No need to close this PR, there are a few things we'll need 👌 |
|
@felixdittrich92 @frgfm The problem is coming from I tested |
|
@felixdittrich92 Yes, you're right, I'll add a small note in requirements.txt |
There was a problem hiding this comment.
Thanks for the fix!
I'll check whether we can afford a looser version specifier (constraining pydantic in this case)
Something I don't get though:
- since the specifier used to be
>=0.65.2, when the docker is built, it was taking the highest version available (I think there is no indirect constraints here), which is 0.79 (0.78 up until recently) - so the only reason I can see that this change fixed the bug, is because the docker couldn't reuse the cache.
Is there something I missed? According to the threads, the faulty fastapi version range is within (0.66 - 0.73).
thanks!
| run: curl http://localhost:8002/docs | ||
| run: wget --spider --tries=12 http://localhost:$PORT/docs |
There was a problem hiding this comment.
Could you remind me the reason of this change? (not familiar with the spider option, the "tries" flag seems explicit)
There was a problem hiding this comment.
spider does only an 'exist' check and avoid downloading anything
There was a problem hiding this comment.
@frgfm Also, curl has an option --retry-all-errors to retry in any case but this option is only available in latest version of curl which can't be (easily) installed on Ubuntu 20.04. So it may still fail for various reasons that we can't catch. However, wget comes out of the box with a "retry on all errors".
There was a problem hiding this comment.
@frgfm Concerning your comment on version, it's a good remark. I didn't changed anything on my side and it didn't install the latest version of fastapi 🤔
This PR aims two solve problems:
curlcommand in CI is run but the API is not fully ready so thePing serverstep often failed. There is an option--retry-all-errorssince curl 7.71.0 but there is only curl 7.68.0 onubuntu-latest(which is ubuntu 20.04) and that's not that simple to update curl to this version.I replaced
curlbywgetwith option--spider.From man page:
fastapiversion should fix the problem.