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

Remove USubscription CreateTopic() and DeprecateTopic() APIs #108

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

stevenhartley
Copy link
Contributor

The topic lifecycle APIs were added as an optimization early on so that subscribers could know if the topic they are subscribing to is valid and the producer is alive however this optimization has given us nothing but heartburn in production as it forces all publishers to call the API when then are initialized and it causes race conditions on initial boot-up. Furthermore, validation of topics could already be done by calling UDiscovery so there was no added benefit of adding the extra states to the subscription service.

#96

The topic lifecycle APIs were added as an optimization early on so that subscribers could know if the topic they are subscribing to is valid and the producer is alive however this optimization has given us nothing but heartburn in production as it forces all publishers to call the API when then are initialized and it causes race conditions on initial boot-up. Furthermore, validation of topics could already be done by calling UDiscovery so there was no added benefit of adding the extra states to the subscription service.

#96
@stevenhartley stevenhartley added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 5, 2024
@stevenhartley stevenhartley added this to the alpha.1 milestone Apr 5, 2024
@stevenhartley stevenhartley self-assigned this Apr 5, 2024
up-l3/usubscription/v3/README.adoc Outdated Show resolved Hide resolved
up-l3/usubscription/v3/README.adoc Outdated Show resolved Hide resolved
up-l3/usubscription/v3/README.adoc Outdated Show resolved Hide resolved
Co-authored-by: Kai Hudalla <kai.hudalla@bosch.com>
Copy link
Contributor

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

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

LGTM

@sophokles73
Copy link
Contributor

@AnotherDaniel this might be of interest to you as well, since you are working on an implementation ...

@stevenhartley stevenhartley merged commit 2ff303c into main Apr 9, 2024
2 checks passed
@lynn2009
Copy link

lynn2009 commented Apr 9, 2024

@stevenhartley , The uSubscription still requires verification of whether the requested topic is valid. By removing the createTopic method, this implies a strong dependency on uDiscovery for its functionality.

@stevenhartley
Copy link
Contributor Author

@stevenhartley , The uSubscription still requires verification of whether the requested topic is valid. By removing the createTopic method, this implies a strong dependency on uDiscovery for its functionality.

@lynn2009 , why do you need to check the topic is valid or not?

@lynn2009
Copy link

lynn2009 commented Apr 10, 2024

If uSubscription does not check the topic is valid or not, we will run into the case the subscriber can subscribe an invalid topic and get success response.

@stevenhartley
Copy link
Contributor Author

If uSubscription does not check the topic is valid or not, we will run into the case the subscriber can subscribe an invalid topic and get success response.

correct

@mayureshkar
Copy link

@stevenhartley @lynn2009 with createTopic removed, what constitutes an 'invalid topic'?

@stevenhartley
Copy link
Contributor Author

invalid UUri

@stevenhartley stevenhartley deleted the createtopic branch May 16, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants