Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated code to use the latest version of PyEnsign and added error handling for closed websocket connections #9

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

pdamodaran
Copy link
Collaborator

The trades app was deployed before changes were made to PyEnsign to support the generator syntax. This PR updates the code to use the latest version of PyEnsign and ensures that the websocket connection is restored if the connection gets closed.

Prior to the websocket connection fix, the app stopped running after encountering this error:

Websocket connection closed: no close frame received or sent

I ran the app overnight and was able to verify that it continues to run even after this error occurred.

@pdeziel - could you also test this on your machine before re-deploying the app? I had to downgrade river to version 0.15.0 due to the issue with the docker build on my machine.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #21689: Update the trades app to use the latest PyEnsign version.

@pdamodaran pdamodaran requested a review from pdeziel October 4, 2023 20:25
Copy link
Collaborator

@pdeziel pdeziel left a comment

Choose a reason for hiding this comment

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

Thanks for updating this! Once the new PyEnsign is released, I will go ahead and version bump and deploy this.

async def handle_ack(ack):
_ = datetime.fromtimestamp(ack.committed.seconds + ack.committed.nanos / 1e9)

async def handle_nack(nack):
print(f"Could not commit event {nack.id} with error {nack.code}: {nack.error}")
logging.error(f"Could not commit event {nack.id} with error {nack.code}: {nack.error}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding the logging!

print(f"Websocket connection closed: {e}")
await asyncio.sleep(1)
logging.error(f"Websocket connection closed: {e}")
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok if I'm reading this correctly continue in this instance is the same as falling through to the next iteration right? So maybe it's not the cause of the app not publishing, but I think this is more clear anyway. Maybe the logging will help us figure out what's going on.

@pdeziel pdeziel merged commit 96e78ac into main Oct 6, 2023
1 check passed
@pdeziel pdeziel deleted the sc-21689 branch October 6, 2023 22:44
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