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

Support customize subscribe.options && compatible with null value for type int #514

Merged

Conversation

cyrushine
Copy link
Contributor

  1. according to https://wamp-proto.org/_static/gen/wamp_latest.html#subscribe-0, message SUBSCRIBE.Options is a dict that allows to provide additional subscription request details in a extensible way
  2. make java type int/long compatible with null value of wamp type int

@oberstet
Copy link
Contributor

thanks for contributing!

rgd custom (implementation defined, rather than official protocol defined) attributes: the library should only accept attribute names x_* for custom attributes .. this isn't yet in the spec, but consensus wamp-proto/wamp-proto#345 and also how (at least) some implementations handle this already .. yeah, sorry, we should really land this in the spec text.

@cyrushine
Copy link
Contributor Author

👌

@oberstet
Copy link
Contributor

oberstet commented Nov 1, 2020

@om26er what do you think rgd the PR? good to merge? in general, the AB libraries should allow users to use custom attributes in "options". such custom options must have attribute names starting with "x_". other attribute names must be from the list of officially supported (eg "exclude_authroles" or similar). do we want to check/enforce that in the ABPy client side library? eg Crossbar.io will kill a session that uses non-standard attribute names that do not start with "x_" - but we certainly could add such checks later in a subsequent change (as this PR doesn't protect from using illegal names ..)

@om26er
Copy link
Contributor

om26er commented Nov 2, 2020

Yep, looks good to me. We can add the protection for illegal names later.

@om26er om26er merged commit a49aff9 into crossbario:master Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants