-
Notifications
You must be signed in to change notification settings - Fork 42
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: reintroduce and deprecate named sharding, make autosharding default for all other cases #1751
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,10 @@ export interface IMetaSetter { | |
} | ||
|
||
export interface EncoderOptions { | ||
/** | ||
* @deprecated | ||
*/ | ||
pubsubTopic?: PubsubTopic; | ||
pubsubTopicShardInfo?: SingleShardInfo; | ||
Comment on lines
+51
to
53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a fan of this redundancy :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danisharora099 This is temporary and a deprecated option. I kept it to allow autosharding support without introducing breaking changes for any existing users. Another option would be to just keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think deprecation period is a good way to go so that we will fully remove the flag in the upcoming update. |
||
/** The content topic to set on outgoing messages. */ | ||
contentTopic: string; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import type { Peer, PeerStore } from "@libp2p/interface/peer-store"; | |
import type { ShardInfo } from "./enr.js"; | ||
import type { CreateLibp2pOptions } from "./libp2p.js"; | ||
import type { IDecodedMessage } from "./message.js"; | ||
import { PubsubTopic } from "./misc.js"; | ||
|
||
export enum Protocols { | ||
Relay = "relay", | ||
|
@@ -26,9 +27,20 @@ export type ContentTopicInfo = { | |
contentTopics: string[]; | ||
}; | ||
|
||
export type ShardingParams = ShardInfo | ContentTopicInfo; | ||
export type ApplicationInfo = { | ||
clusterId: number; | ||
application: string; | ||
version: string; | ||
}; | ||
|
||
export type ShardingParams = ShardInfo | ContentTopicInfo | ApplicationInfo; | ||
|
||
export type ProtocolCreateOptions = { | ||
/** | ||
* @deprecated | ||
* Waku will stop supporting named sharding. Only static sharding and autosharding will be supported moving forward. | ||
*/ | ||
pubsubTopics?: PubsubTopic[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adklempner can you, please, create a task with description of what should be removed to completely deprecate parameters/functionality? So that we address it in a skip release or something There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened #1775 |
||
/** | ||
* Waku supports usage of multiple pubsub topics. This is achieved through static sharding for now, and auto-sharding in the future. | ||
* The format to specify a shard is: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering is
this.initializePubsubTopic
can be used here or adjusted to be usedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.initializePubsubTopic
is used to set the pubsub topics for theFilter
object, while thisconst pubsubTopic
is used to get/set the pubsub topic for theSubscription
object.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated ternary expression to be less verbose in latest commit