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

[3007.x] Add started event to PublishServer #66994

Merged
merged 6 commits into from
Oct 29, 2024

Conversation

dwoz
Copy link
Contributor

@dwoz dwoz commented Oct 24, 2024

  • Add a started event that gets set after the server transport is ready for clients to connect.
  • Wait on publish servers start while the master process is starting up.

What does this PR do?

What issues does this PR fix or reference?

Fixes #66993

Previous Behavior

Publish clients in the master process could encounter exceptions while trying to connect.

New Behavior

Publish clients within the master processes connect without error.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

- Add a started event that gets set after the server transport is ready
for clients to connect.
- Wait on publish servers start while the master process is starting up.
twangboy
twangboy previously approved these changes Oct 25, 2024
twangboy
twangboy previously approved these changes Oct 25, 2024
@dwoz
Copy link
Contributor Author

dwoz commented Oct 29, 2024

  1. I'm not sure the rational for passing this as an argument. What's the use case for using an arbitrary object? I think it would be better to have a separate method that you can give a timeout to so that transports can implement their own methodology and better integrate with [3007.x] transports: fix and normalize PublishServer method signatures #66751. Something like def wait_startup(self, timeout): bool
  2. I think [3007.x] transport: tcp check that pull_path exists before chmod #66931 is needed for the tests to fully pass.

I agree a method like wait_startup might make sense in the future.

@dwoz dwoz merged commit 9d10d69 into saltstack:3007.x Oct 29, 2024
279 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:full Run the full test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants