Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

check for system-wide installation of nodes; dynamically populate node-menu #1064

Closed
wants to merge 16 commits into from

Conversation

luclu
Copy link
Contributor

@luclu luclu commented Aug 3, 2016

closes #264

This current state of this PR will make getNodePaths.js to:

  • getNodePaths.probe()
    • enumerate bundled nodes
    • check if system-installation of the relevant nodes are available (only Linux and mac)
    • gather and compare their versions
    • store the relevant (latest version's) paths in a resolvedPaths[type] => {type:{path:version}} dictionary
    • takes about 800ms with bundled + system instances of geth + eth (macbook pro, SSD)
  • getNodePaths.query(nodeType)
    • returns preferred path of node

Prefers the latest version of a chosen node. Also system installation of node has higher priority.

TODO

  • abstract to make drag and drop addition of any node possible (like parity)
  • make develop and production compatible
  • use promises
    • to collect external command's outputs
      • refactor promise structure (race conditions)
    • to integrate with ethereumNode.js
  • debug log entries
  • start probe() as soon as initial node connection times out (don't wait the 3 seconds)
    • promisify to prevent timeout
  • basic error handling
  • populate node menu labels dynamically
    • add version string
    • hide if only one node type is available
    • initiate rebuild of appMenu (gives an error); don't wait for node startup to trigger repopulate
  • rebase on Integrating electron-builder for better distributables building #972 and Downloading node client binaries at runtime #1228 once merged

@alexvandesande
Copy link
Collaborator

How's the progress here Luca? You seem to be doing some interesting stuff

@luclu luclu changed the title closes #264, check for system wide installation of nodes check for system-wide installation of nodes; dynamically populate node-menu Sep 24, 2016
@5chdn
Copy link

5chdn commented Sep 27, 2016

Tested on ArchLinux. 👍

@luclu luclu closed this Sep 30, 2016
@luclu luclu deleted the check-syste-nodes branch September 30, 2016 09:51
@luclu luclu restored the check-syste-nodes branch September 30, 2016 09:51
@luclu luclu reopened this Sep 30, 2016
@evertonfraga
Copy link
Member

@luclu any status on this PR? i can test along with #1228 if needed.

@luclu
Copy link
Contributor Author

luclu commented Oct 24, 2016

@hiddentao what functionality this PR provides is not available via the new node-manager and thus should be added to it if desirable?

@hiddentao
Copy link
Contributor

@luclu The drag-and-drop support for Parity - how exactly will that work?

The new client-binaries library would be able to scan for download Parity binaries. But at present we have no UI for letting the user choose what client they wish to use. I was wondering if adding this is a good idea - e.g. we can provide a command-line flag which tells Mist to first show a window showing available clients for the system (availability calculated using client-binaries). The user would then be able to choose which client they want to use for that run. Of course, one issue with doing this is that since Geth and Parity use different chaindata and keystore folders the user will not be able to seamlessly pick up where they left off with the previous client in terms of chain sync.

What do you think?

@luclu
Copy link
Contributor Author

luclu commented Nov 28, 2016

@hiddentao I think all functionality of this PR was superseded by the current implementation of the clientBinaryManager.

I will close this.

@luclu luclu closed this Nov 28, 2016
@lock
Copy link

lock bot commented Mar 31, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Mar 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Look for a system wide installation of geth or eth
5 participants