Skip to content

Fix false 'headers not set' warning on DELETE endpoints#424

Merged
brandon-wada merged 1 commit intomainfrom
fix/request-id-empty-headers
Apr 15, 2026
Merged

Fix false 'headers not set' warning on DELETE endpoints#424
brandon-wada merged 1 commit intomainfrom
fix/request-id-empty-headers

Conversation

@brandon-wada
Copy link
Copy Markdown
Collaborator

@brandon-wada brandon-wada commented Apr 14, 2026

Summary

  • The check if not header_param: in GroundlightApiClient.call_api incorrectly treats an empty dict {} as falsy, skipping X-Request-Id injection and logging a spurious warning for every DELETE endpoint (delete_detector, delete_priming_group, delete_rule, reset_detector) and edge_report_metrics_create
  • These endpoints have empty accept and content_type in their OpenAPI-generated headers_map, so no Accept/Content-Type headers are added, leaving params["header"] as {}
  • Fix: change if not header_param: to if header_param is None: so an empty dict falls through to the elif branch and gets the X-Request-Id header set

🤖 Generated with Claude Code

Brandon: Without having to understand the entire generated code path, we can reason that this code change is an improvement. If headers is an empty dict, we still want to inject the request id.
This code is ancient, there's plenty we could improve here but this PR will stick to the minimal change to fix the bug that proper usage can create a warning without much to do about it and cause the request id to not be set

…headers not set"

The check `if not header_param:` incorrectly treats an empty dict {} as
falsy, skipping X-Request-Id injection for every endpoint whose OpenAPI
spec has empty accept and content_type headers. This affects all DELETE
endpoints (delete_detector, delete_priming_group, delete_rule,
reset_detector) and edge_report_metrics_create.

The fix changes the guard to `if header_param is None:` so that an empty
dict correctly falls through to the elif branch and gets the request-id
header set.
Copy link
Copy Markdown
Contributor

@timmarkhuff timmarkhuff left a comment

Choose a reason for hiding this comment

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

LGTM.

I question whether it should be en error instead of warning. "This will never happen in normal usage." sounds like an error to me.

@brandon-wada brandon-wada merged commit 43d5599 into main Apr 15, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants