Skip to content
New issue

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

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

Already on GitHub? # to your account

Implement services (ADR-32) #566

Merged
merged 33 commits into from
Aug 23, 2024
Merged

Implement services (ADR-32) #566

merged 33 commits into from
Aug 23, 2024

Conversation

caspervonb
Copy link
Collaborator

@caspervonb caspervonb commented Jun 10, 2024

This implements services (ADR-32), this is based on a stripped down version of this implementation by @charbonnierg.

This does not have a "client" or "monitor" type like nats.deno, we can come back and add a higher level interface for it later if it is needed. For the first pass it aims for functional parity with nats.go.

Also adds compatibility tests.

Closes #552

@caspervonb caspervonb force-pushed the services branch 5 times, most recently from 00c7672 to 8e4a023 Compare June 11, 2024 08:51
@caspervonb caspervonb changed the title [WIP] Implement services (ADR-32) Implement services (ADR-32) Jun 11, 2024
@caspervonb caspervonb marked this pull request as ready for review June 11, 2024 08:51
@caspervonb caspervonb requested a review from wallyqs June 11, 2024 08:54
@wallyqs
Copy link
Member

wallyqs commented Jun 20, 2024

Think may have to update these: https://github.com/nats-io/nats.py/blob/main/setup.py#L14

@swelborn
Copy link

swelborn commented Jul 1, 2024

Thanks for making this -- appears to be working well! I do have a static checker issue with micro.add_service:

"add_service" is not exported from module "nats.micro"

Adding that to the list of exports in micro.__init__.py removes it.

__all__ = [
    "api",
    "control_subject",
    "Verb",
    "Endpoint",
    "Group",
    "Handler",
    "Request",
    "Service",
    "add_service"
]

tests/test_micro.py Outdated Show resolved Hide resolved
@swelborn
Copy link

swelborn commented Jul 3, 2024 via email

@mgale
Copy link

mgale commented Jul 3, 2024

I noticed an issue with nats-surveyor trying to parse the messages that go through the service, not sure where the failure is or if this works with the nats.go.

[WARN] unsupported service observation received for id: /observations/obs.json, service name: myservice, kind: io.nats.unknown_message

From the suryeor code:
https://github.com/nats-io/nats-surveyor/blob/main/surveyor/observation.go#L358

nats/micro/request.py Outdated Show resolved Hide resolved
@caspervonb caspervonb force-pushed the services branch 3 times, most recently from 2d71992 to 3dc54ed Compare July 18, 2024 11:33
@caspervonb caspervonb requested review from Jarema and wallyqs July 22, 2024 22:35
Copy link
Member

@aricart aricart left a comment

Choose a reason for hiding this comment

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

LGTM - only a question on some error messages

if self.queue_group:
if not SUBJECT_REGEX.match(self.queue_group):
raise ValueError(
"Invalid queue group. Queue group must not contain spaces, and can only have '>' at the end."
Copy link
Member

Choose a reason for hiding this comment

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

not sure what > is at the name of a queue group - this is copy/paste?

if self.subject:
if not SUBJECT_REGEX.match(self.subject):
raise ValueError(
"Invalid subject. Subject must not contain spaces, and can only have '>' at the end."
Copy link
Member

Choose a reason for hiding this comment

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

is this error for the internal service subjects?

@caspervonb caspervonb force-pushed the services branch 3 times, most recently from ef0a1cf to 29428a7 Compare August 22, 2024 10:54
caspervonb and others added 3 commits August 22, 2024 19:04
Co-Authored-By: Guillaume Charbonnier <guillaume.charbonnier@araymond.com>
Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@wallyqs wallyqs merged commit 1649118 into main Aug 23, 2024
1 of 2 checks passed
@wallyqs wallyqs deleted the services branch August 23, 2024 03:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Service API
5 participants