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

Client URL compatibility with go client? #33

Closed
markphelps opened this issue Apr 11, 2019 · 1 comment
Closed

Client URL compatibility with go client? #33

markphelps opened this issue Apr 11, 2019 · 1 comment

Comments

@markphelps
Copy link

markphelps commented Apr 11, 2019

We have a Twirp service implemented in Go and are trying to use this client to make requests in our Ruby codebase.

It seems that the Ruby client (I have only tried the protobuf version so far), does not add /twirp before the service name/method when constructing the URL.

As shown in your examples:
https://github.com/twitchtv/twirp-ruby/blob/1cecfd1b7f1990c650a5209de5b93cd1b4a99b37/example/hello_world_client.rb#L6

The user has to add the /twirp path parameter when setting the URL for the client.

This seems to be at odds with the Go client, which seems to construct that path as the Go server expects.

Example:
https://github.com/twitchtv/twirp/blob/ad6d075411e462f08e971d7efc18f4a770ac8b40/example/cmd/client/main.go#L27

As a user, I would expect that both clients operate in a similar manner, and that you would only have to set the address of your server when setting the URL with the client and not have to make sure that you add the /twirp segment.

@marioizquierdo
Copy link
Collaborator

marioizquierdo commented Apr 12, 2019

The general path setup has been proposed in go Twirp a while ago:

The Ruby implementation was done already in protocol v6 to avoid having to do backwards compatible changes later. The only problem is that v6 is taking more than a year in Go to see the light 😅.

So, if you want your service to be v5 compatible, simply mount it with /twirp prefix. In the future that prefix will no longer be mandatory.

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

No branches or pull requests

2 participants