Updates due dependency version changes. #1960
Conversation
h5py recently changes and now values once returned as str are now returned as bytes. This would have caused a much larger change, so instead the version is restricted to <3.0.0. This caused the bulk of the testing failues. A few other changes were needed to make a few other tests pass. #1959
Codecov Report
@@ Coverage Diff @@
## main #1960 +/- ##
=======================================
Coverage ? 65.61%
=======================================
Files ? 52
Lines ? 4196
Branches ? 0
=======================================
Hits ? 2753
Misses ? 1443
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| cls.mock_oauth_process.start() | ||
| server_okay = False | ||
| for _ in range(5): |
There was a problem hiding this comment.
Can you add a comment explaining why we need to retry in order for this setup to work?
There was a problem hiding this comment.
Oops -- sorry I guess my comment wasn't clear. I was more wondering what necessitates the for _ in range(5): line (i.e. why do we need to retry). Does it take up to 5 seconds for the mock oauth server to be ready?
There was a problem hiding this comment.
All this is doing is polling until the mock server is ready. The issue is we are starting a mock oauth server, then we are starting a cellxgene server, which will start making requests to the mock oauth server. So there is a race condition because the mock oauth server needs to be ready before it gets requests. So, we check to see if it is ready, and if not we wait 1 second, then try again. If it gets to 5 seconds, which is shouldn't, we assume something has gone wrong and fail the test.
There was a problem hiding this comment.
Ah got it -- this makes sense to me. Mind writing the same explanation as a comment above the for loop? That way, the next reader knows why we need that retry there.
@maniarathi Is it good now, or did you want the longer comment? |
|
Sorry I meant that I'd like to see your explanation to me about the race condition between the two servers and that the mock Auth0 server shouldn't take more than 5 seconds added to the inline comment in code. Then LGTM. |
|
Thanks for double checking! |
* Updates due dependency version changes. h5py recently changes and now values once returned as str are now returned as bytes. This would have caused a much larger change, so instead the version is restricted to <3.0.0. This caused the bulk of the testing failues. A few other changes were needed to make a few other tests pass. #1959
h5py recently changes and now values once returned as str are now returned as bytes.
This would have caused a much larger change, so instead the version is restricted to <3.0.0.
This caused the bulk of the testing failues.
A few other changes were needed to make a few other tests pass.
#1959