Pattern-based registrations/subscriptions

[DZabavchik]

Resolves #46, #95, Answers #2

• Visual Studio 2015 solution and projects (Updated .gitignore)
• invocation and event provide actual URI from message to callee/handler.
• Fixes for test projects (tested with crossbar 0.12.1)

[oberstet]

the amount of stuff to ignore for VS is ridiculous

[oberstet]

also, to test, I need working examples again first. see #98

[jpetso]

You're not using the new parameter in any examples in this commit, it's undocumented and I'm not clear what it's purpose is. I'd be hesitant to merge more complicated API without having a clear view of what benefits it brings, and that those benefits outweigh the complexity and loss of readability that an additional boolean brings here. Please explain.

[oberstet]

Yep. This PR combines multiple things (and it's easier to merge focused PRs). And it's unclear to be too what intermediate is for ..

[DZabavchik]

Will revoke PR, and resubmit as multiple PRs.

VS project samples can be removed and kept on a fork. But since they are for samples only not the main project no harm in keeping them.

Sorry about tabs, not a fan of tabs myself. Will fix.

Intermediate is a provision for progressive calls. Will remove and submit as separate PR when complete and tested.

On Feb 19, 2016, at 09:00, Tobias Oberstein notifications@github.com wrote:

In autobahn/wamp_invocation.hpp:

@@ -195,13 +200,13 @@ class wamp_invocation_impl
* Reply to the invocation with positional arguments.
*/
template

• void result(const List& arguments);
• void result(const List& arguments, bool intermediate = false);
  Yep. This PR combines multiple things (and it's easier to merge focused PRs). And it's unclear to be too what intermediate is for ..

—
Reply to this email directly or view it on GitHub.

[oberstet]

@DZabavchik all in all, I would like to definitely merge from this PR the following:

• the things for subscribe options
• the guarding for non-existing UDS on Windows

I am also ok with merging all the VS specific stuff like the Git ignore and VS project/build files (because that's probably what Windows dev would expect .. not cmake, though that should work on Windows too).

My main issue hence is with: bool intermediate, because it introduces new API.

Could you explain?

Is that supposed to be something for progressive call results?

[oberstet]

The merge conflict now is because the subscribe options and CHALLENGE (missing break) bugs were fixed .. only a few conflicting lines.

[DZabavchik]

I guess it was a case of simultaneous discovery. I went through the all of the samples except for UDS, and made sure they work. Fixed the subscribe, and so did David.

The missing break that was fixed in PR 91, went missing again after epic merge for transport abstraction.

Regards,

Denis

On Feb 19, 2016, at 10:05, Tobias Oberstein notifications@github.com wrote:

The merge conflict now is because the subscribe options and CHALLENGE (missing break) bugs were fixed .. only a few conflicting lines.

—
Reply to this email directly or view it on GitHub.

[oberstet]

@DZabavchik sounds good! Thanks!

[DZabavchik]

@oberstet,

re: sounds good!

Should I cancel/close the PR?

[oberstet]

@DZabavchik Ah right .. as you said you would file follow up PRs, I closed this one. Pls note that I also just merged @davidchappelle PR #100 .. if you could pull that so that you don't waste your time with potential conflicts. Thanks for contributing!

[oberstet]

@DZabavchik one more note regarding progressive call results. Instead of expanding the invocation callback with individual parameters (like a progressive result callback), we might consider adding just 1 parameter: a call details (and for pubsub, an event details) object. That object can then hold a progressive result callback, but can also hold details like caller session ID (for caller disclosure) and the like. More extensible. At least that's how AutobahnPython and AutobahnJS have it ..

[DZabavchik]

we still need to differentiate between intermediate results (and set progress:true), and final results. In addition we need to take into consideration whether caller is expecting/requested progressive results. There is already set_details on invocation that copies URI, same method can look at "receive_progress": true. Result callback should either discard intermediate results when caller did not request them, or throw.

[davidchappelle]

@DZabavchik https://github.com/DZabavchik I was also working on adding
support for progressive call results. We probably had something similar I
haven't looked at your branch yet. But if you are working on it, it might
be worth taking a look at what I had. I hadn't gotten to the part where I
needed to handle the progressive results on the callee side. I'll push what
I had in an hour or two and let you know which branch it is on.

On Fri, Feb 19, 2016 at 11:06 AM, Denis Zabavchik notifications@github.com
wrote:

we still need to differentiate between intermediate results (and set
progress:true), and final results. In addition we need to take into
consideration whether caller is expecting/requested progressive results.
There is already set_details on invocation that copies URI, same method can
look at "receive_progress": true. Result callback should either discard
intermediate results when caller did not request them, or throw.

—
Reply to this email directly or view it on GitHub
#97 (comment)
.

[DZabavchik]

@davidchappelle, I just noticed that progressive and pattern-based registrations issue were unresolved for quite some time. Was under impression that you are pushing the transport part forward (was hoping to see an example with websockets from you :) No need for competition and duplicate work. Please check pattern_registration_subs branch. It has a clean version of this failed PR with no windows stuff. Let me know (I removed progressive calls provision from there)

[jpetso]

Another thing I wanted to mention, for the intermediate bool, is that it should likely be an enum instead of boolean. It looks nicer if I call, say,

invocation.result(std::tuple(), autobahn::result_progress);

than the one from the PR, which looks like this:

invocation.result(std::tuple(), true); // do I really have to add comments to write readable code?

[DZabavchik]

Missed the comment, sry.

In that case, wouldn't explicit intermediate_result() or progress() that use private method internally be a better choice?

Regards,

Denis

On Feb 19, 2016, at 17:43, Jakob Petsovits notifications@github.com wrote:

Another thing I wanted to mention, for the intermediate bool, is that it should likely be an enum instead of boolean. It looks nicer if I call, say,

invocation.result(std::tuple(), autobahn::result_progress);
than the one from the PR, which looks like this:

invocation.result(std::tuple(), true); // do I really have to add comments to write readable code?
—
Reply to this email directly or view it on GitHub.

[jpetso]

Actually yes, that would be much better API :)
Go for it!

[jpetso]

Personally, I like progress() better, not only is it shorter and more intuitive, but the name also matches up with what the specification calls it.

[DZabavchik]

@jpetso done. See PR #105