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

Listen directly to Broker for messages #2500

Merged
merged 9 commits into from
Mar 24, 2023

Conversation

luketomlinson
Copy link
Contributor

@luketomlinson luketomlinson commented Mar 22, 2023

This PR adds 3 things:

  1. BrokerHttpClient (just the client to call the /message endpoint)
  2. BrokerServer (following the *Server pattern. This maintains a connection with the server and uses the http client to make the requests)
  3. BrokerMessageListener (wraps the BrokerServer and continuously polls for messages)

We will set the ServerUrlV2 in a followup PR.

@luketomlinson luketomlinson changed the title Luketomlinson/broker clients Listen directly to Broker for messages Mar 23, 2023
@luketomlinson luketomlinson marked this pull request as ready for review March 23, 2023 18:14
@luketomlinson luketomlinson requested a review from a team as a code owner March 23, 2023 18:14
}
else
{
// more aggressive backoff [30, 60]
Copy link
Contributor

Choose a reason for hiding this comment

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

Basically we'll retry forever in case of retriable exceptions?
Nit: Regarding comment, It's less aggressive right? since we're increasing the retry intervals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I think so. This is copied behavior from the existing Message listener

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it could be read as "more aggressively backing off by increasing the time" 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have some maximum of continuousError where I abort if it reaches that

@luketomlinson luketomlinson merged commit 92258f9 into main Mar 24, 2023
@luketomlinson luketomlinson deleted the luketomlinson/broker-clients branch March 24, 2023 18:00
nikola-jokic pushed a commit to nikola-jokic/runner that referenced this pull request May 12, 2023
AdamOlech pushed a commit to antmicro/runner that referenced this pull request Aug 14, 2024
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