search

waku-org / nwaku

Waku node and protocol.
Other
184 stars 47 forks source link

chore: remove json rpc #2416

Closed alrevuelta closed 3 months ago

alrevuelta commented 3 months ago

Description

⚠️ Completely removes the json-rpc api, removing also the following cli flags:

From now on, to interact with nwaku refer to the REST API. Closes https://github.com/waku-org/nwaku/issues/2053

github-actions[bot] commented 3 months ago

This PR may contain changes to configuration options of one of the apps.

If you are introducing a breaking change (i.e. the set of options in latest release would no longer be applicable) make sure the original option is preserved with a deprecation note for 2 following releases before it is actually removed.

Please also make sure the label release-notes is added to make sure any changes to the user interface are properly announced in changelog and release notes.

github-actions[bot] commented 3 months ago

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2416

Built from 7d77d2dabf2560ed03a5c74b0d75dce2d8c7a92c

Ivansete-status commented 3 months ago

Thanks for this!

However, IMO, I think we first need to have a mature REST and I'm afraid we are not in that stage yet. On the other hand, we will need to coordinate and inform infra about that deprecation.

IMO, the best strategy would be:

  1. Complete REST and make sure the documentation and all endpoints work perfectly.
  2. Inform infra about our intention of deprecating json-rpc
  3. Merge this PR
alrevuelta commented 3 months ago

Complete REST and make sure the documentation and all endpoints work perfectly.

@Ivansete-status mind being more actionable? whats missing in the rest API?

Inform infra about our intention of deprecating json-rpc

done

Ivansete-status commented 3 months ago

@Ivansete-status mind being more actionable? whats missing in the rest API?

Sure! These are the most relevant outstanding points, without considering bug issues.

Private Rest API - https://github.com/waku-org/nwaku/issues/2227 Reorganize RestApi specs for live documentation - https://github.com/waku-org/nwaku/issues/2120 Reorganize rest-api types - https://github.com/waku-org/nwaku/issues/2121 Allow to configure RLN via the REST API - https://github.com/waku-org/nwaku/issues/1985

As a reminder, REST tasks were deprioritized in favor of others.

@chair28980 - this is something that we could comment on in our next pm meeting and give more priority to that matter.

alrevuelta commented 3 months ago

Private Rest API - https://github.com/waku-org/nwaku/issues/2227 Reorganize RestApi specs for live documentation - https://github.com/waku-org/nwaku/issues/2120 Reorganize rest-api types - https://github.com/waku-org/nwaku/issues/2121 Allow to configure RLN via the REST API - https://github.com/waku-org/nwaku/issues/1985

In order:

alrevuelta commented 3 months ago

Relevant https://github.com/waku-org/pm/issues/125#issuecomment-1926727368

Ivansete-status commented 3 months ago
  • Lets move the conversations of this to feat: HTTP REST API: Private API #2227
  • Don't see how that's a blocker, isn't enough with this?
  • Seems like a refactor to me, why blocker?
  • No RLN endpoints are present neither in the json rpc, why would that be a blocker?

in order:

1.- Ok!

2.- 3.- 4.- I agree that these points seem not to be blockers at all but I'd like someone (me or another nwaku engineer) to invest some time in ensuring:

On the other hand, we cannot deprecate JSON-RPC abruptly without proper notification to counterparties. i.e. we need to inform that we are starting to deprecate the JSON-RPC and keep this notification for at least two consecutive releases. In other words, we need to start informing other teams/projects to start using REST and once we make sure (assuming 2 releases is enough time) they can work well with REST, then it is fine to decommission JSON-RPC.

And btw, thanks again @alrevuelta for this initiative! I think is very interesting to have it completed!

@chair28980 @gabrielmer @NagyZoltanPeter - I've added a point for tomorrow's PM session to discuss it

NagyZoltanPeter commented 3 months ago

Private Rest API - #2227

This one is missing although it was not part of json-rpc so nothing outstanding.

Reorganize RestApi specs for live documentation - #2120

With new targets of go-waku, it will be easier, due to former msalignment between the two implementations. Reorg is done in a separate repo: https://github.com/waku-org/waku-rest-api We need to remove specs from nwaku and go-waku to let live them in one place.

Reorganize rest-api types - #2121

Yes, this is aiming a small refactoring.

Allow to configure RLN via the REST API - #1985

This one also new stuff.

In order:

  • Lets move the conversations of this to feat: HTTP REST API: Private API #2227
  • Don't see how that's a blocker, isn't enough with this?
  • Seems like a refactor to me, why blocker?
  • No RLN endpoints are present neither in the json rpc, why would that be a blocker?

I see nothing blocking here in order to remove json-rpc.

NagyZoltanPeter commented 3 months ago

Btw: rpcPrivate flag is not used anywhere.

alrevuelta commented 3 months ago

As agreed, we will announce the deprecation in the current release 0.25.0 and completely remove support in 0.26.0. This PR is blocked till then.

NagyZoltanPeter commented 3 months ago

@jakubgs please note that API docs had moved and living live here: https://waku-org.github.io/waku-rest-api/#get-/debug/v1/info

Version info:

curl -X GET "http://127.0.0.1:8645/debug/v1/version" -H "accept: text/plain"
jakubgs commented 3 months ago

I see, didn't know that. But if the docs exist there shouldn't the obsolete/outdated one in docs/api be removed?

Also, shouldn't we publish https://waku-org.github.io/waku-rest-api/ under https://rest-api.waku.org/ or https://api.waku.org/?

jakubgs commented 3 months ago

Here you go:

NagyZoltanPeter commented 3 months ago

I see, didn't know that. But if the docs exist there shouldn't the obsolete/outdated one in docs/api be removed?

Also, shouldn't we publish https://waku-org.github.io/waku-rest-api/ under https://rest-api.waku.org/ or https://api.waku.org/?

Yes, I agree we will need to find a more straightforward place for it instead of a gh pages hosted one. But notice REST API works got de-prio in favor of other task. For the doc reference page we will need to cross check any difference in order to remove specs from nwaku / go-waku repos.

LordGhostX commented 3 months ago

@NagyZoltanPeter, thanks for the ping 🫡

I removed references to the JSON-RPC API a long time ago in preference for the REST API. I'll take down the config section once it's released: https://docs.waku.org/guides/nwaku/config-options#json-rpc-config

alrevuelta commented 3 months ago

With https://github.com/status-im/infra-role-nim-waku/pull/25 merged and js-waku-node CI job ✅ , merging. js-waku-node-optional is a known flaky one.