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: remove named topic #844

Merged
merged 7 commits into from
Oct 30, 2023
Merged

feat: remove named topic #844

merged 7 commits into from
Oct 30, 2023

Conversation

harsh-98
Copy link
Contributor

@harsh-98 harsh-98 commented Oct 27, 2023

Description

Named pubsubTopic is being deprecated. Only default and shared pubsubTopic will be supported. In reference to #842

Changes

  • update go-waku examples.
  • Remove PubsubTopic() from mobile, c and node libraries.
  • Only support Default pubsub and sharded pubsubTopic.

Tests

  • update pubSubTopic test.

@status-im-auto
Copy link

status-im-auto commented Oct 27, 2023

Jenkins Builds

Click to see older builds (36)
Commit #️⃣ Finished (UTC) Duration Platform Result
e92307f #1 2023-10-27 10:30:47 ~1 min linux 📄log
✔️ e92307f #1 2023-10-27 10:31:42 ~2 min nix-flake 📄log
✖️ e92307f #1 2023-10-27 10:32:39 ~3 min tests 📄log
✔️ e92307f #1 2023-10-27 10:33:40 ~4 min android 📦tgz
✔️ e92307f #1 2023-10-27 10:34:09 ~4 min ios 📦tgz
✖️ e92307f #1 2023-10-27 10:36:25 ~6 min tests 📄log
✖️ d6941f5 #2 2023-10-27 10:38:56 ~28 sec tests 📄log
✔️ d6941f5 #2 2023-10-27 10:40:08 ~1 min linux 📦deb
✔️ d6941f5 #2 2023-10-27 10:40:22 ~1 min nix-flake 📄log
✖️ d6941f5 #2 2023-10-27 10:40:39 ~2 min tests 📄log
✔️ d6941f5 #2 2023-10-27 10:41:44 ~3 min ios 📦tgz
✔️ d6941f5 #2 2023-10-27 10:42:18 ~3 min android 📦tgz
✔️ 8e549f7 #3 2023-10-27 10:43:20 ~1 min nix-flake 📄log
✔️ 8e549f7 #3 2023-10-27 10:43:39 ~2 min tests 📄log
✔️ 8e549f7 #3 2023-10-27 10:43:53 ~2 min linux 📦deb
✔️ 8e549f7 #3 2023-10-27 10:45:08 ~3 min tests 📄log
✔️ 8e549f7 #3 2023-10-27 10:45:31 ~4 min ios 📦tgz
✔️ 8e549f7 #3 2023-10-27 10:46:09 ~3 min android 📦tgz
✔️ 829fb02 #4 2023-10-28 06:28:36 ~1 min linux 📦deb
✔️ 829fb02 #4 2023-10-28 06:29:35 ~2 min nix-flake 📄log
✔️ 829fb02 #4 2023-10-28 06:30:52 ~3 min tests 📄log
✔️ 829fb02 #4 2023-10-28 06:31:08 ~3 min tests 📄log
✔️ 829fb02 #4 2023-10-28 06:31:56 ~4 min android 📦tgz
✔️ 829fb02 #4 2023-10-28 06:32:43 ~5 min ios 📦tgz
✔️ 1d83665 #5 2023-10-28 06:29:45 ~1 min linux 📦deb
✔️ 1d83665 #5 2023-10-28 06:31:35 ~1 min nix-flake 📄log
✔️ 1d83665 #5 2023-10-28 06:32:39 ~4 min ios 📦tgz
✔️ 1d83665 #5 2023-10-28 06:33:09 ~1 min tests 📄log
✔️ 1d83665 #5 2023-10-28 06:34:16 ~3 min tests 📄log
✔️ 1d83665 #5 2023-10-28 06:35:41 ~3 min android 📦tgz
✔️ 02e410c #6 2023-10-30 10:44:03 ~1 min tests 📄log
✔️ 02e410c #6 2023-10-30 10:44:07 ~1 min nix-flake 📄log
✔️ 02e410c #6 2023-10-30 10:44:30 ~2 min tests 📄log
✔️ 02e410c #6 2023-10-30 10:44:34 ~2 min linux 📦deb
✔️ 02e410c #6 2023-10-30 10:46:02 ~3 min android 📦tgz
✔️ 02e410c #6 2023-10-30 10:46:30 ~4 min ios 📦tgz
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 0f9bd24 #7 2023-10-30 12:53:38 ~3 min nix-flake 📄log
✔️ 0f9bd24 #7 2023-10-30 12:53:48 ~3 min android 📦tgz
✔️ 0f9bd24 #7 2023-10-30 12:54:00 ~4 min linux 📦deb
✔️ 0f9bd24 #7 2023-10-30 12:55:19 ~5 min tests 📄log
✔️ 0f9bd24 #7 2023-10-30 12:55:52 ~5 min tests 📄log
✔️ 0f9bd24 #7 2023-10-30 13:06:23 ~16 min ios 📦tgz
✔️ 1ff9a1a #8 2023-10-30 14:02:35 ~2 min nix-flake 📄log
✔️ 1ff9a1a #8 2023-10-30 14:02:41 ~2 min tests 📄log
✔️ 1ff9a1a #8 2023-10-30 14:02:51 ~2 min tests 📄log
✔️ 1ff9a1a #8 2023-10-30 14:03:08 ~2 min linux 📦deb
✔️ 1ff9a1a #8 2023-10-30 14:04:39 ~4 min android 📦tgz
✔️ 1ff9a1a #8 2023-10-30 14:08:55 ~8 min ios 📦tgz

@harsh-98 harsh-98 marked this pull request as ready for review October 27, 2023 10:45
Copy link
Member

@richard-ramos richard-ramos left a comment

Choose a reason for hiding this comment

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

🚀

waku/v2/protocol/pubsub_topic.go Outdated Show resolved Hide resolved
library/mobile/api.go Outdated Show resolved Hide resolved
@richard-ramos
Copy link
Member

richard-ramos commented Oct 27, 2023

I'm wondering if we should we also replace the topic string by a WakuPubsubTopic in

cc: @chaitanyaprem

EDIT: Hmm... thinking more on this, I think we should change the protocols to WakuPubsubTopic wherever strings are used, but do it later, because if we do that now, we'd be automatically dropping support to named pubsub topics and this would mean that TheGraph wouldn't be able to use go-waku without changing their pubsub topic structure.
We should keep support for string pubsub topics for now

@chaitanyaprem
Copy link
Collaborator

I'm wondering if we should we also replace the topic string by a WakuPubsubTopic in

* https://github.com/waku-org/go-waku/blob/8e549f7049037f4ea7778695aa78b2926e1bc2f8/waku/v2/protocol/relay/waku_relay.go#L227

* https://github.com/waku-org/go-waku/blob/8e549f7049037f4ea7778695aa78b2926e1bc2f8/waku/v2/protocol/lightpush/waku_lightpush_option.go#L43

* as well as wherever a pubsub topic is used in filter and store protocol?

cc: @chaitanyaprem

EDIT: Hmm... thinking more on this, I think we should change the protocols to WakuPubsubTopic wherever strings are used, but do it later, because if we do that now, we'd be automatically dropping support to named pubsub topics and this would mean that TheGraph wouldn't be able to use go-waku without changing their pubsub topic structure. We should keep support for string pubsub topics for now

I remember Hanno mentioning that we are going to still have legacy API's so that existing users like TheGraph are not affected. Instead of modifying existing API's, we should be having new API's with deprecation note mentioned for existing API's that take pubsubTopic string.

waku/v2/protocol/pubsub_topic.go Show resolved Hide resolved
library/c/api.go Outdated Show resolved Hide resolved
waku/v2/protocol/content_topic.go Outdated Show resolved Hide resolved
harsh-98 and others added 3 commits October 28, 2023 13:27
Co-authored-by: richΛrd <info@richardramos.me>
Co-authored-by: richΛrd <info@richardramos.me>
@harsh-98 harsh-98 merged commit ddf188b into master Oct 30, 2023
2 checks passed
@harsh-98 harsh-98 deleted the RemoveNamedTopic branch October 30, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants