-
Notifications
You must be signed in to change notification settings - Fork 127
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: Flag to fully disable pubsub and all cross-node syncing #3286
Conversation
a1b0ead
to
4cd0434
Compare
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.
Simpler than I would have expected.
@@ -498,7 +497,7 @@ export class Dispatcher { | |||
* @param tip - Commit CID | |||
*/ | |||
publishTip(streamId: StreamID, tip: CID, model?: StreamID): Subscription { | |||
if (process.env.CERAMIC_DISABLE_PUBSUB_UPDATES == 'true' || EnvironmentUtils.useRustCeramic()) { | |||
if (process.env.CERAMIC_DISABLE_PUBSUB_UPDATES == 'true' || !this.enableSync) { | |||
return empty().subscribe() |
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.
what is the difference between CERAMIC_DISABLE_PEER_DATA_SYNC and CERAMIC_DISABLE_PUBSUB_UPDATES ?
I take it this means we could just not publish, or we could not publish and not read or sync with other nodes? but neither applies under rust ceramic?
If we were going to keep this i think it woudl be confusing but if we are going to leave js-ceramic entirely soon i guess it is fine.
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.
CERAMIC_DISABLE_PUBSUB_UPDATES
is much more limited in scope. It just disables publishing of update messages. We'll still listen to pubsub and still publish query and response messages.
Honestly it's not really very useful, we put it in once a while ago to try to troubleshoot an issue gitcoin was having, I don't think anyone uses it anymore and we could frankly probably get rid of it.
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.
LGTM, one comment about possibly confusing option combinations, but given that we're trying to transition away anyway i think its fine
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.
Flag can be enabled by setting
ipfs.disablePeerDataSync
in the config file, or with the newCERAMIC_DISABLE_PEER_DATA_SYNC
env var.turning on this flag does 3 things:
Blocks published to ipfs with this flag enabled should still be fetchable by other ipfs nodes over bitswap.