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

feat!: determine pubsubTopic based on contentTopic #59

Merged
merged 7 commits into from
Apr 16, 2024

Conversation

weboko
Copy link
Collaborator

@weboko weboko commented Apr 15, 2024

Problem

In latest fleet deploy there is no support of default pubsubtopic
Which means now it should be determined based on contentTopic

@weboko weboko requested a review from adklempner April 15, 2024 23:51
@weboko
Copy link
Collaborator Author

weboko commented Apr 15, 2024

@adklempner what do you think about this approach?

@weboko weboko merged commit 814512c into master Apr 16, 2024
4 of 5 checks passed
@weboko weboko deleted the weboko/pubsub-contenttopic branch April 16, 2024 00:09
Comment on lines -80 to +81
return (
"/" + qr.applicationName + "/" + qr.applicationVersion + "/wakunoise/1/sessions_shard-" + qr.shardId + "/proto"
);
return "/" + qr.applicationName + "/" + qr.applicationVersion + "/" + qr.shardId + "/proto";
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason behind this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is to align with convention towards contentTopic as stated in here https://github.com/vacp2p/rfc-index/blob/main/waku/informational/23/topics.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if it is not followed - then we won't be able to map it to shards when autosharding is default

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.

2 participants