Skip to content

Conversation

@bougyman
Copy link
Member

@bougyman bougyman commented Aug 6, 2025

  • fix: Allows non-blocking setup_worker method
  • fix: Ensure we do not block in non-blocking mode

This allows running specs and any other tasks
that you want to accomplish without being blocked
by worker threads sleeping

Copilot AI review requested due to automatic review settings August 6, 2025 18:42
@bougyman bougyman added the enhancement New feature or request label Aug 6, 2025
@bougyman bougyman self-assigned this Aug 6, 2025
@bougyman bougyman requested a review from BryceARich August 6, 2025 18:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes blocking behavior in the NATS API server to allow non-blocking operation at the instance level. The changes enable running specs and other tasks without being blocked by worker threads sleeping.

  • Adds a blocking parameter throughout the worker spawn chain to control blocking behavior
  • Splits worker setup into blocking (setup_worker!) and non-blocking (setup_worker) variants
  • Refactors parameter passing to use keyword arguments for better clarity

Comment on lines +118 to +120
return worker.setup_worker!(nats_url: url, service_opts: opts) if blocking

worker.setup_worker(nats_url: url, service_opts: opts)
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The conditional logic is inverted. When blocking is true, the method calls setup_worker! which blocks with sleep, but then immediately returns, preventing the sleep from executing. This should be unless blocking or the logic should be restructured.

Suggested change
return worker.setup_worker!(nats_url: url, service_opts: opts) if blocking
worker.setup_worker(nats_url: url, service_opts: opts)
if blocking
worker.setup_worker!(nats_url: url, service_opts: opts)
else
worker.setup_worker(nats_url: url, service_opts: opts)
end

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it got this wrong. Not sure why it thinks the sleep is prevented from executing

@client = NATS.connect url
@service = @client.services.add(**opts)
@client = NATS.connect nats_url
@service = @client.services.add(build_service_opts(service_opts:))
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The build_service_opts method call uses named parameter syntax but the method definition shows service_opts: parameter. This should be build_service_opts(service_opts: service_opts) for clarity.

Suggested change
@service = @client.services.add(build_service_opts(service_opts:))
@service = @client.services.add(build_service_opts(service_opts: service_opts))

Copilot uses AI. Check for mistakes.
@bougyman bougyman merged commit 01748a5 into main Aug 6, 2025
4 checks passed
@bougyman bougyman deleted the allow-nonblocking branch August 6, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants