Skip to content

Add check if headers is not None before treating it like a dict#1909

Merged
gareth-ellis merged 1 commit intomasterfrom
improve-resiliency
Feb 3, 2025
Merged

Add check if headers is not None before treating it like a dict#1909
gareth-ellis merged 1 commit intomasterfrom
improve-resiliency

Conversation

@gareth-ellis
Copy link
Copy Markdown
Member

Martijn came across an issue with ccr-stats, where we are updating headers, but we don't have any request headers set - so this bit of code blows up. In other cases (e.g blob-store-stats) we typically run against serverless, which we specifically don't try and update the headers for.

@gareth-ellis gareth-ellis added the enhancement Improves the status quo label Jan 31, 2025
@gareth-ellis gareth-ellis requested a review from a team January 31, 2025 16:56
Copy link
Copy Markdown
Member

@martijnvg martijnvg 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 can confirm that this fix addresses the error with ccr-stats telemetry device.

# Converts all parts of a Accept/Content-Type headers
# from application/X -> application/vnd.elasticsearch+X
mimetype = request_headers.get(header, None)
mimetype = request_headers.get(header, None) if request_headers else None
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.

LGTM. By the way I think the following would be more clear to read:

if request_headers:
  mimetype  = request_headers.get(header, None)
  if mimetype:
     ...

@gareth-ellis gareth-ellis merged commit cd2eaca into master Feb 3, 2025
@gareth-ellis gareth-ellis deleted the improve-resiliency branch February 3, 2025 10:35
@dpifke-elastic dpifke-elastic added this to the 2.12.0 milestone Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improves the status quo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants