Skip to content

Conversation

@SionoiS
Copy link
Contributor

@SionoiS SionoiS commented Mar 3, 2025

update to the Waku sync spec to include full topic support.

@SionoiS SionoiS requested a review from jm-clius March 3, 2025 21:14
@SionoiS SionoiS self-assigned this Mar 3, 2025
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

LGTM. Comment to address before merging below - we have to be explicit about "topic" as being pubsub topic, content topic or both. Suggestion is to limit spec only to behaviour for pubsub topic, which will allow it to undergo full testing and QA separately to reach draft status while we experiment with content topics in parallel (which IIUC may necessitate more comprehensive storage changes).

@SionoiS SionoiS requested review from a team and removed request for a team March 20, 2025 19:28
Copy link
Contributor

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

Thanks for it!
I think we should avoid using the term pubsub outside the nwaku codebase.
From API pov, I'd only use these terms:

  • timestamp
  • cluster-id
  • shard
  • content-topic

@SionoiS
Copy link
Contributor Author

SionoiS commented Mar 31, 2025

Thanks for it! I think we should avoid using the term pubsub outside the nwaku codebase. From API pov, I'd only use these terms:

  • timestamp
  • cluster-id
  • shard
  • content-topic

I don't have strong opinions here we can either;

  • use pubsub and content topics
  • use cluster, shard and content topics

I think @jm-clius want the spec to be agnostic to network definitions?

@jm-clius
Copy link
Contributor

I don't have strong opinions here we can either;

  • use pubsub and content topics
  • use cluster, shard and content topics

I think @jm-clius want the spec to be agnostic to network definitions?

Most of our core level specs use pubsub_topic as the mapping to pubsub topic from cluster ID and shard is a higher-level function. That said, I'm OK with use moving to cluster ID + shard throughout this spec, as long as we're consistent and do not refer to any specific deployment (e.g. TWN)

@SionoiS
Copy link
Contributor Author

SionoiS commented Apr 1, 2025

Most of our core level specs use pubsub_topic as the mapping to pubsub topic from cluster ID and shard is a higher-level function. That said, I'm OK with use moving to cluster ID + shard throughout this spec, as long as we're consistent and do not refer to any specific deployment (e.g. TWN)

I'm lazy and I don't want to change it back to shard again so I'll leave it like this.

@SionoiS
Copy link
Contributor Author

SionoiS commented Apr 10, 2025

I removed cluster and left only pubsub and content topics

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.

4 participants