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

#35 topic wildcard handling #40

Merged
merged 2 commits into from
Jan 15, 2021
Merged

#35 topic wildcard handling #40

merged 2 commits into from
Jan 15, 2021

Conversation

GreenRover
Copy link
Contributor

@GreenRover GreenRover commented Dec 19, 2020

Issue #3: Replace wildcards from topic name to _ in queue name

@Nephery
Copy link
Collaborator

Nephery commented Dec 21, 2020

Thanks @GreenRover. I'll take a deeper look at this next year to consider for the upcoming 2.0.0 release. But here are my initial thoughts:

Issue 2: Enable to have variables in topic name.

I think this a neat idea. My only concern about this, is that if somebody actually wanted a subscription called sensor/temperature/<station>. Then it wouldn't work since <station> would be detected as a variable instead of a plain string.

Instead, to resolve this conflict, this should be a configurable feature which the user can decide to use or not use. e.g. create a boolean producer/consumer binding config option named something along the lines of useDestinationWildcardVariables. Not entirely sure if this feature should be enabled or disabled by default.

Will get a header "destination_station" == "bern"

Instead of creating a different header for each variable, It might be cleaner to create a single header called something like destinationVariables or destinationWildcards and have it be of type Map<String,String> where an entry's key is the variable name (station in this case) and its value is the variable value (bern in this case).


@Mrc0113 could I also get your thoughts on this when you're back next year?

@GreenRover
Copy link
Contributor Author

is not a valid part of a subscription. I choosed the <> because > is a solace wildcard

@Nephery
Copy link
Collaborator

Nephery commented Dec 22, 2020

A > that appears anywhere else other than by itself at the last level of a subscription topic in the string is treated as the > character rather than a wildcard.

https://docs.solace.com/PubSub-Basics/Wildcard-Charaters-Topic-Subs.htm

So in this particular case, the > in sensor/temperature/<station> would be treated as a literal character.

@GreenRover
Copy link
Contributor Author

Instead of creating a different header for each variable, It might be cleaner to create a single header called something like destinationVariables or destinationWildcards and have it be of type Map<String,String> where an entry's key is the variable name (station in this case) and its value is the variable value (bern in this case).

I changed it to: "destinationVariables"

@GreenRover
Copy link
Contributor Author

GreenRover commented Jan 4, 2021

Rebased to: "#37" waiting for it to be merged.

@Nephery
Copy link
Collaborator

Nephery commented Jan 6, 2021

@GreenRover could you split issue 2 into a separate PRs?

@GreenRover
Copy link
Contributor Author

@Nephery This pr only contains_ enable the destination to be able to contain wild cards.
And topic variables was moved to #45

@Nephery Nephery changed the base branch from master to stage-2.0.0 January 7, 2021 18:16
Copy link
Collaborator

@Nephery Nephery left a comment

Choose a reason for hiding this comment

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

Needs tests to ensure that everything that depends on queue names works with the substitution:

  • Basic consumer group functionality
  • Producer-side consumer group pre-provisioning (i.e. requiredGroups)
  • Additional topic subscriptions (i.e. queueAdditionalSubscriptions)
  • Error queue republishing

Also I suggest rebasing your changes onto the squashed commit in stage-2.0.0 (#37 has been merged):
git rebase --onto stage-2.0.0 HEAD~<commit_count>

@GreenRover
Copy link
Contributor Author

  • rebased to stage-2.0.0 done
  • Test: "Basic consumer group functionality" manually done
  • Test: "Producer-side consumer group pre-provisioning" i dont understand what "requiredGroups" is
  • Test: "Additional topic subscriptions" manually done
  • Test: "Error queue republishing" manually done

@Nephery Nephery linked an issue Jan 15, 2021 that may be closed by this pull request
@Nephery Nephery merged commit c1ad273 into SolaceProducts:stage-2.0.0 Jan 15, 2021
carolmorneau added a commit that referenced this pull request Mar 15, 2023
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

Successfully merging this pull request may close these issues.

[SOL-43239] Enhance destination Wildcard Handling in SCSt Binder
2 participants