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

Enable pinning when the ipfs daemon is in offline mode #790

Open
rehno-lindeque opened this issue Oct 13, 2019 · 14 comments
Open

Enable pinning when the ipfs daemon is in offline mode #790

rehno-lindeque opened this issue Oct 13, 2019 · 14 comments
Labels
exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked
Milestone

Comments

@rehno-lindeque
Copy link

Currently ipfs companion is completely disabled when ipfs is run in offline mode. I.e.

ipfs daemon --offline

However, it should still be possible to pin content when not connected to the network. That is

  • This Page > Add to IPFS (Keep Filename)
  • This Page > Add to IPFS
  • Tools > Open Web UI

Should all be available. Also possibly

  • Tools > Share files via IPFS

For this last menu option, the wording "Share" may introduce some confusion. On the other hand, presumably, someone that is savvy enough to run with --offline probably already understands that sharing will only take effect once they put the daemon into online mode.

@lidel
Copy link
Member

lidel commented Oct 16, 2019

Thank you for filling this. It compliments #782 nicely, enabling users to add/browse content in truly offline mode.

The fix is to enable Web UI item in browser action menu when peer count is 0.
I believe all is needed to enable offline mode is replace isIpfsOnline (peers > 0) with isApiAvailable (peers >= 0) in two places:

https://github.com/ipfs-shipyard/ipfs-companion/blob/8512691b59020e1403d6157e53f6b18acde91a81/add-on/src/popup/browser-action/tools.js#L18

https://github.com/ipfs-shipyard/ipfs-companion/blob/8512691b59020e1403d6157e53f6b18acde91a81/add-on/src/popup/browser-action/context-actions.js#L30

@rehno-lindeque is this something you would be interested in contributing via a PR?
I'd be happy to provide more context if needed.

@lidel lidel added exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature good first issue Good issue for new contributors help wanted Seeking public contribution on this issue labels Oct 16, 2019
@rehno-lindeque
Copy link
Author

I'd love to be able to contribute to ipfs related projects but unfortunately I can't justify it for the foreseeable future. Thank you for the suggestion though.

@lidel lidel added exp/beginner Can be confidently tackled by newcomers kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked and removed exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature labels Apr 5, 2020
@BrianBelhumeur
Copy link

👋🏻 I'll take this one

@lidel
Copy link
Member

lidel commented Jul 15, 2020

@BrianBelhumeur sweet!
I believe the code did not change much so entry points described in #790 (comment) should be still valid.

@lidel lidel added this to the v2.15 milestone Sep 6, 2020
@lidel
Copy link
Member

lidel commented Oct 19, 2020

@BrianBelhumeur hi, are you still interested in this, or can someone else pick this up?

@BrianBelhumeur
Copy link

HI @lidel thanks for checking in... I got hung up on some issues I had running the project and couldn't get to the point of actually completing this issue. There were a number of packages that were not listed in package.json that are required to run the plugin, and even after installing those packages by hand, the plugin could still never find my local IPFS instance, running or not. I'm happy to open an issue addressing this if you think it could indeed be an issue with the project, but it felt a bit more to me like something local to my machine. The list of packages that were missing is:

dlv
ipns
libp2p
libp2p-bootstrap
libp2p-delegated-content-routin
libp2p-delegated-peer-routing
libp2p-gossipsub
libp2p-kad-dhta
libp2p-mdns
libp2p-secio
libp2p-websockets
libp2p-websocket-star-multi
pull-mplex

@lidel
Copy link
Member

lidel commented Oct 19, 2020

@BrianBelhumeur I suspect you are building with npm? Try running npm run dev-build, it will use yarn internally and ensure all dependencies are in place.

@lidel lidel modified the milestones: v2.15, v2.16 Oct 19, 2020
@BrianBelhumeur
Copy link

@lidel That seems to have worked, thank you! Not sure what I was doing before, I thought I had followed the instructions in the readme. 🤷🏻‍♂️ I did have to install the utility jq independently, but that's it.

I'll continue to work on this ticket. If anyone wishes to take it from me, feel free because I don't want to be a blocker. It might take me a bit because I'm new to the codebase.

Cheers!

@lidel
Copy link
Member

lidel commented Oct 20, 2020

@BrianBelhumeur no worries, we can keep pushing this feature to a later release 👍

Codebase changed a bit since 2019 so my old comments may not be accurate anymore, but most likely removing "isIpfsOnline" requirement from places like the line below is enough (all we need is extension to be active and API to be available):

https://github.com/ipfs-shipyard/ipfs-companion/blob/5f5d9a63e26b6e224b44f6d42deb5b3f76476004/add-on/src/popup/browser-action/context-actions.js#L44

You may search for places where isIpfsOnline is used and remove it if its impacting features listed in #790 (comment)

Its ok to open a PR if it is not 100% complete, we can land it together :)

ps. if you want to run "offline" node for testing and have access to Docker, below command will create one in an ephemeral container:

docker run --rm -it --net=host ipfs/go-ipfs:v0.7.0 daemon --offline

@BrianBelhumeur
Copy link

Thank @lidel that's very kind and helpful of you! Here's my progress so far:

  • Tools > Quick Import/Share... and Tools > Go to My Node... are enabled in offline mode. That was very straightforward as outlined above
  • The context menu seems to be controlled by a different file than mentioned. I see add-on/src/lib/context-menus.js controlling whether the right-click This Page > Import to IPFS is enabled or not. The menu items in add-on/src/lib/context-actions.js are different, and I haven't been able to discover where they appear.
  • Within add-on/src/lib/context-menus.js, the menu items are [en/dis]abled based in info in getState() which derives info from add-on/src/lib/state.js. Because isApiAvailable is derived in other places by calling browser.runtime.getBackgroundPage(), I'm not sure if that is available in the context of state.js (I haven't tried yet) or if there's a good place to inject info from store in the call chain of the context menu creation ipfs-companion.js createContextMenus() => context-menus.js

I'll open a [WIP] PR next, and your help in landing it would be greatly appreciated. Thanks again!

@BrianBelhumeur
Copy link

I can't seem to push my branch... do I need permissions to do so?

@lidel
Copy link
Member

lidel commented Oct 22, 2020

@BrianBelhumeur I believe you need to (1) fork this repo (2) open PR from your fork

As for context-menus.js, it is already executing in the "background page" context and seem to be fine with zero peers(?). But just for consistency try replacing getState().peerCount >= 0 with getState().peerCount > -1

@BrianBelhumeur
Copy link

@lidel PR created above, however it is not in a working state.

Whether the IPFS daemon is actually not running, or running in offline mode, state.active is true and browser.runtime.getBackgroundPage() is available in both cases. These are the criteria that the store uses to determine isApiAvailable (see https://github.com/ipfs-shipyard/ipfs-companion/blob/master/add-on/src/popup/browser-action/store.js#L276). Additionally, peerCount is -1 in both cases as well.

Given the above, I'm not sure what criteria we can use to determine the difference between offline mode and true unavailability of the daemon. The state object looks identical under both conditions. The only difference I see is the error message generated by apfs-companion.js line 470:

Daemon turned off:
Error while ipfs.swarm.peers: TypeError: NetworkError when attempting to fetch resource.

Daemon running in offline mode:
Error while ipfs.swarm.peers: HTTPError: this action must be run in online mode, try running 'ipfs daemon' first

@lidel
Copy link
Member

lidel commented Oct 23, 2020

Thank you @BrianBelhumeur, I've refactored things a bit in your PR (#936) and added a way to differentiate between "api is down" and "there are no peers but api is up" states.

The remaining work needs to be addressed upstream (ipfs/ipfs-webui#853 (comment)), but should not block your PR from being merged.

@lidel lidel modified the milestones: v2.16, v2.17 Nov 18, 2020
@lidel lidel modified the milestones: v2.17, v2.18 Jan 11, 2021
@galargh galargh moved this to To do in IPFS-GUI (PL EngRes) Sep 22, 2022
@lidel lidel modified the milestones: v2.18, v2.21 Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked
Projects
No open projects
Status: Needs Grooming
Development

No branches or pull requests

3 participants