Skip to content

Conversation

@sendhil
Copy link
Contributor

@sendhil sendhil commented Feb 24, 2025

If you attempt to publish a message and there's a connection error the attempt at logging the error message throws an exception because session and or response could be None. This change accounts for this by declaring the variables.

Test Plan

You'll need to make sure your config file doesn't set lattice-ip or lattice-bearer-token

  1. Run the sample without this change and verify the output is full of spammy exceptions
  2. Run the sample after this change and verify that the output isn't as noisy. Here's a before and after - https://gist.github.com/sendhil/27c6ac08f186f6aea5534c5367ac8cf2

@sendhil sendhil requested a review from a team as a code owner February 24, 2025 06:22
@sendhil
Copy link
Contributor Author

sendhil commented Feb 24, 2025

Before (Left), After (Right)
CleanShot 2025-02-23 at 22 11 43@2x

@sendhil
Copy link
Contributor Author

sendhil commented Feb 24, 2025

Apologies, meant to submit this with my other changes Thursday but didn't get around to it until just now.

Raises:
None
"""
session: Optional[aiohttp.ClientSession] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The auto formatter obscures some of the changes here, but the changes are effectively these 2 lines, and the changes to line 76-77.

except Exception as error:
self.logger.error(
f"lattice api publish entity error {error}\n\trequest={session.request}\n\tresponse={response.status}\n\t{entity}")
f"lattice api publish entity error {error}\n\trequest={session.request if session else None}\n\tresponse={response.status if response else None}\n\t{entity}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The auto formatter obscures some of the changes here, but the changes are effectively these 2 lines, and the changes to line 62-63.

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.

1 participant