Skip to content

fix(conf): report legacy gRPC URLs as deprecations - #1276

Merged
dapr-bot merged 6 commits into
dapr:mainfrom
kocaemre:fix/1275-grpc-endpoint-deprecation
Oct 16, 2026
Merged

dapr-bot merged 6 commits into
dapr:mainfrom
kocaemre:fix/1275-grpc-endpoint-deprecation

Conversation

@kocaemre

@kocaemre kocaemre commented Oct 13, 2026 •

Copy link
Copy Markdown

Description

  • Report legacy http:// and https:// gRPC endpoints with a caller-attributed DeprecationWarning, instead of a default-visible UserWarning.
  • Show the equivalent host, port and TLS query in the message; keep endpoint parsing and TLS behavior unchanged.
  • Test both schemes for the exact single warning category, caller location, parsed settings and conflicting TLS query.

AI assistance was used to develop and verify this change.

Issue reference

Closes #1275

Test plan

  • Initial RED: 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).
  • Review RED (mutation probe): temporarily injected an extra 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.
  • GREEN: 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 mypy passed on the initial revision; not rerun after test-only follow-up.

Checklist

  • Code compiles correctly (unit suite)
  • Created/updated tests
  • Extended the documentation (no public API or supported endpoint format change)

@kocaemre
kocaemre requested review from a team as code owners October 13, 2026 18:27
@CasperGN CasperGN self-assigned this Oct 13, 2026

@CasperGN CasperGN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#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>
@kocaemre
kocaemre force-pushed the fix/1275-grpc-endpoint-deprecation branch from 5736b12 to b61e873 Compare October 13, 2026 22:53
@kocaemre

Copy link
Copy Markdown
Author

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.

@CasperGN CasperGN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2 of 3: follow-up on the changes since 5736b12.

Every earlier blocker is resolved and the new changes add none.

Resolved since the last round: 1.

Drafted with AI assistance and reviewed by me.

@sicoyle sicoyle added automerge allows dapr bot to merge a PR after tests pass autoupdate backport release-1.19 labels Oct 15, 2026
@dapr-bot
dapr-bot enabled auto-merge October 15, 2026 14:03
@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (27f1158) to head (cc88403).
⚠️ Report is 1 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kocaemre

Copy link
Copy Markdown
Author

CI follow-up: validate (3.11) failed before tests during ollama pull llama3.2:latest with 503: upstream connect error or disconnect/reset before headers (run 38753056637, Set up Llama). Python 3.10/3.12/3.13/3.14 validation and lint passed; validate-complete failed because the 3.11 leg failed. I tried gh run rerun 38753056637, but GitHub reports that this run cannot be rerun from my session. Could a maintainer rerun the validate workflow when the model registry is available? No source change seems indicated by this log.

@dapr-bot
dapr-bot added this pull request to the merge queue Oct 16, 2026
Merged via the queue into dapr:main with commit 3ab5d06 Oct 16, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge allows dapr bot to merge a PR after tests pass autoupdate backport release-1.19

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report the http(s):// gRPC endpoint deprecation as a DeprecationWarning, not a UserWarning on every client

4 participants