Issue 267: Remove runtime drop error message #311
Conversation
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
==========================================
- Coverage 75.63% 68.88% -6.76%
==========================================
Files 45 26 -19
Lines 11351 6170 -5181
==========================================
- Hits 8585 4250 -4335
+ Misses 2766 1920 -846
|
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
| use std::net::Ipv4Addr; | ||
|
|
||
| #[test] | ||
| #[serial] |
There was a problem hiding this comment.
I don't understand why these tests can't be run in parallel. This is testing config values.
If it's modifying environment variables, can we split that out into its own test so it don't affect others too much?
There was a problem hiding this comment.
Yes it's because the env var is a shared resource and running unit tests in parallel will have issues.
I have been googling around and cannot find a better way to deal with shared resource like env var in the unit test. Either we run it consecutively or run it using only 1 thread.
|
Thanks @Tristan1900 . Regarding the build failing for prost version 0.7 on rust rustc 1.56.0-nightly , I have moved the build to use the stable version of rustc in PR: #312 |
|
@shrids Thanks Sandeep. I guess we can keep the Tonic upgrade in this PR since we will need to upgrade at some point in the future anyway. |
| token: Option<String>, | ||
| } | ||
|
|
||
| impl Interceptor for AuthInterceptor { |
There was a problem hiding this comment.
Could you elaborate more on this change? I think it is unrelated to the PR.
There was a problem hiding this comment.
Yes, it's the necessary change for upgrading Tonic version. I am following the example here
python_binding/tox.ini
Outdated
| @@ -13,6 +13,6 @@ deps = | |||
| pytest | |||
| pytest-timeout | |||
| aiounittest | |||
| commands = pytest {posargs:tests} --timeout=300 -vvvvv | |||
| commands = pytest {posargs:tests} --timeout=300 -vvvvv -s | |||
There was a problem hiding this comment.
-s disables capturing, capturing simplifies debugging if the tests fail.
There was a problem hiding this comment.
I see. I found that without -s we couldn't see the log printed out to the stdout, so it's not easy to debug when the tests are stuck at some place.
I can revert it back
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
Signed-off-by: Wenqi Mou <wenqi.mou@dell.com>
Change log description
Remove runtime drop error message.
Upgrade tonic version to 0.5, prost version to 0.8 as prost version 0.7 is having problem with
rustc 1.56.0-nightly (50171c310 2021-09-01)ref.Purpose of the change
Fix #267 #308 #253
What the code does
ClientFactory is the only place that contains the Runtime and it cannot be cloned, so there is only one place that's holding the runtime now.
Make another struct ClientFacotryAsync that holds
Runtime::Handle, which can be cloned and used in async context.How to verify it
Test should pass