feat: enable pubsub via settings menu #1735
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@lidel, can you please have a look as you're able? |
Without this the setting won't be applied. License: MIT Signed-off-by: Marcin Rataj <[email protected]>
ipfsd.stop() did not wait for proper shutdown, which caused things to go racy during situations where we restart node. When unlucky, I ended up with two go-ipfs processses. Bumping version to include fix from ipfs/js-ipfsd-ctl#554 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
License: MIT Signed-off-by: Marcin Rataj <[email protected]>
already logged in ./src/utils/create-toggler.js, so here we only log statud when app starts and that is all License: MIT Signed-off-by: Marcin Rataj <[email protected]>
lidel
changed the title
it is still an experiment in go-ipfs: https://github.com/ipfs/go-ipfs/blob/master/docs/experimental-features.md#ipfs-pubsub License: MIT Signed-off-by: Marcin Rataj <[email protected]>
lidel
approved these changes
Jan 19, 2021
There was a problem hiding this comment.
@andrew looks and works great! ❤️
- Moved PubSub toggle to experiments, because it is marked as one in go-ipfs docs:
https://github.com/ipfs/go-ipfs/blob/master/docs/experimental-features.md#ipfs-pubsub - Fixed the duplicated message in 47f83cb
- We have to restart go-ipfs to have the flagg applied, so I've added code that triggers restart via
ipcMain.emit('ipfsConfigChanged')- While doing that found a bug in logic responsible for restarting the node on config changes. Took me a while to realize where the duplicated process comes from.
- Ended up bumping
ipfsd-ctl, as the bug seems to be fixed in fix: kill disposable nodes on stop and simplify started status js-ipfsd-ctl#554 - 👉 It works as expected on Linux, mind testing on macOS before I merge this?
- Toggling Enable PubSub should restart the node now.
- Standalone Restart action in the main menu should also work
|
What if we enabled pubsub by default 🙀 💞 |
|
@olizilla if we do it here, we also should enable it in Brave (:lion: → 🙀) I'd rather wait for ipfs/kubo#5383 and ipfs/kubo#6621 😅 |
🚂 🚋🚋🚋
|
Wow, thanks for the updates @lidel, I was only expecting some pointers, not just for you to finish the pr! Tested on macOS, working as expected, restarting correctly and such 🎉 |
License: MIT Signed-off-by: Marcin Rataj <[email protected]>
|
@andrew I would give pointers but then found the unrelated bug, and it escalated 😅 |
|
Retested on macOS, still looking good 👍 |
lidel
added a commit
that referenced
this pull request
Jan 25, 2021
* feat: disable/enable gc via settings menu Closes #1647 Very similar approach to #1735 * fix: ensure checkbox follows cli flag config - setting default state to on for new and pre-existing config - remove keysize (not needed since go-ipfs 0.7.0 uses ED25519 keys by default) Co-authored-by: Marcin Rataj <[email protected]>
lidel
added a commit
that referenced
this pull request
Jan 25, 2021
* feat: enable ipns over pubsub via settings menu Part of #1647 Very similar approach to #1735 * refactor: move pubsub experiments to exp. section making it easier to find if someone edits config by hand Co-authored-by: Marcin Rataj <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #1647
Adds a menu preference option to enable pubsub which adds a flag to the ipfsd-ctl options for startup.
Question: should this prompt a restart, given that the setting won't take effect until ipfs has been been restarted?
Bug: I'm getting the enabled/disabled message logged twice when clicking on the menu item, not quite sure why :