-
Notifications
You must be signed in to change notification settings - Fork 490
swarm deploy refactored to use web3.bzz instead of command line #372
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
Conversation
* `Embark.upload()` refactored to build own `Engine` and services so `web3` could be passed to `Swarm` module * `Swarm.deploy()` modified to use `web3.bzz.upload()` * needs detection of running swarm node
lib/index.js
Outdated
| engine.startService("pipeline"); | ||
| engine.startService("codeGenerator"); | ||
| engine.startService("deployment"); | ||
| engine.startService("ipfs"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
he suggestion is:
- add here
engine.startService("swarm")
and add the relevant code to engine.js (see ipfs e.g)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
lib/index.js
Outdated
| // 2. upload to storage (outputDone event triggered after webpack finished) | ||
| self.events.on('outputDone', function () { | ||
| cmdPlugin.uploadCmds[0].cb() | ||
| cmdPlugin.uploadCmds[0].cb({web3: engine.web3}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- remove
{web3: engine.web3}
| class Swarm { | ||
| constructor(options) { | ||
| this.options = options; | ||
| this.buildDir = options.buildDir || 'dist/'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- add this.web3 and this.storageConfig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correction/addition, it should actually be in swarm/index.js (which is the actual module initialization), then the module can pass the params to upload
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
lib/modules/swarm/upload.js
Outdated
| } | ||
|
|
||
| callback(null, swarm_bin); | ||
| function findWeb3(callback){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- remove findWeb3
lib/modules/swarm/upload.js
Outdated
| callback(stderr, {code: code, output: stdout}); | ||
| }); | ||
| function setProvider(callback){ | ||
| web3.bzz.setProvider(`http://${self.options.storageConfig.host}:${self.options.storageConfig.port}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- initialize based on storageConfig params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a code change in this file? Or was this comment here for clarity of process/flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine, although I think it could simply be this.storageConfig and that value be assigned on the constructor (so this method is not coupled with this.options)
* Removed config init from `cmd.js` for upload. * refactored `upload()` to use engine services instead of loading and using plugins directly. * now passing web3 directly to the `Swarm` constructor
| embarkConfig: 'embark.json', interceptLogs: false | ||
| }); | ||
| _options.env = environment; | ||
| _options.env = env || 'development'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove the initConfig? in fact I don't quite understand how this.config.storageConfig would work later without this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instantiations of Events/Logger/Config objects are done in both Embark.initConfig() and in Engine.init(). In this case, Embark.upload method added an Engine.init() and therefore no longer need to run Embark.initConfig() as it was duplicating effort.
You'll also notice in the Embark.upload after the Engine is instantiated, the Engine's events/logger/config properties are used instead of those local to Embark as initConfig is no longer called.
lib/core/engine.js
Outdated
| this.registerModule('swarm', { | ||
| addCheck: this.servicesMonitor.addCheck.bind(this.servicesMonitor), | ||
| storageConfig: this.config.storageConfig, | ||
| host: _options.host, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is _options.host and _options.port coming from the cmd line, if so, they are not specified, does commander automatically include them in the options? regardless we should add those parameters in cmds.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually these are unused, and have been removed now. The host and port (used later in the swarm module) come from the storage config. Are you sure you want me to add these as command line options?
Embark.upload()refactored to build ownEngineand services soweb3could be passed toSwarmmoduleSwarm.deploy()modified to useweb3.bzz.upload()