Repository navigation
fix(conf): report legacy gRPC URLs as deprecations - #1276
Conversation
CasperGN
left a comment
There was a problem hiding this comment.
Thanks @kocaemre for picking this up, and for the contribution to the Dapr Python SDK! 🙌 Switching to a DeprecationWarning with stacklevel fixes the noisy log line without touching parsing, and showing the host:port?tls=... equivalent for the URL the user actually passed is a nice touch. I left one small comment inline about a test #1275 asks for, please take a look. 🧪
Drafted with AI assistance and reviewed by me.
| def test_http_scheme_deprecation_keeps_endpoint_and_points_to_caller(self): | ||
| for scheme, port, secure in [('http', 80, False), ('https', 443, True)]: | ||
| with self.subTest(scheme=scheme): | ||
| with self.assertWarns(DeprecationWarning) as warning: |
There was a problem hiding this comment.
#1275 also asks for a test showing no UserWarning is raised. assertWarns(DeprecationWarning) still passes if a UserWarning is emitted alongside it, so it doesn't cover that. What do you think about recording all warnings and checking the categories? Something like:
with warnings.catch_warnings(record=True) as caught:
warnings.simplefilter('always')
GrpcEndpoint(f'{scheme}://host:{port}')
self.assertEqual([w.category for w in caught], [DeprecationWarning])That checks both the category and that only one warning is raised per parse.
Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com>
5736b12 to
b61e873
Compare
|
Thanks for spotting the gap. Added a separate exact-category warning regression for both http and https in signed-off b61e873. RED (temporary extra UserWarning probe): 2 expected subtest failures, with [UserWarning, DeprecationWarning] vs [DeprecationWarning]; probe removed. GREEN: helpers file 3 passed/4 subtests; non-e2e unit suite 2248 passed/41 deselected/65 subtests; touched-file Ruff check/format and git diff --check passed. Updated PR test plan with commands. No new UserWarning is emitted. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1276 +/- ##
=======================================
Coverage 86.98% 86.98%
=======================================
Files 163 163
Lines 12996 12997 +1
=======================================
+ Hits 11305 11306 +1
Misses 1691 1691 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI follow-up: |
Description
http://andhttps://gRPC endpoints with a caller-attributedDeprecationWarning, instead of a default-visibleUserWarning.AI assistance was used to develop and verify this change.
Issue reference
Closes #1275
Test plan
uv run --frozen pytest tests/conf/helpers_test.py -q -k test_http_scheme_deprecation_keeps_endpoint_and_points_to_caller --tb=short -o addopts=""→ 2 expected subtest failures (DeprecationWarning not triggered).UserWarning;uv run --frozen pytest tests/conf/helpers_test.py -q -k test_http_scheme_emits_only_a_deprecation_warning --tb=short -o addopts=""→ 2 expected subtest failures ([UserWarning, DeprecationWarning]vs[DeprecationWarning]). Removed probe before commit.uv run --frozen pytest tests/conf/helpers_test.py -q -o addopts="" --tb=short→ 3 passed, 4 subtests passed.uv run --frozen pytest tests -m "not e2e" --ignore=tests/integration --ignore=tests/examples -q -o addopts="" --tb=short→ 2248 passed, 41 deselected, 65 subtests passed (pre-existing warnings).uv run --frozen ruff check tests/conf/helpers_test.py dapr/conf/helpers.py;uv run --frozen ruff format --check tests/conf/helpers_test.py dapr/conf/helpers.py;git diff --check origin/main..HEAD→ pass.uv run --frozen mypypassed on the initial revision; not rerun after test-only follow-up.Checklist