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

Missing changes required to make new UI work #2793

Merged
merged 9 commits into from
Oct 22, 2016
Merged

Missing changes required to make new UI work #2793

merged 9 commits into from
Oct 22, 2016

Conversation

tomusdrw
Copy link
Collaborator

  1. Remove old dapps
  2. Redirect 8080->8180
  3. Proxy home.parity to http://127.0.0.1:8180
  4. Enabled CORS for /api to allow request from signer port (if signer is enabled)
  5. Additional tracing for Dapps and Signer

One thing left:
When on http://home.parity connection to WS is broken (browser is sending some unhadled requests to WS-RS server, assuming it's a proxy server)

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 21, 2016
Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good, a couple questions and comments.

@@ -850,7 +849,7 @@ dependencies = [
[[package]]
name = "jsonrpc-http-server"
version = "6.1.1"
source = "git+https://github.com/ethcore/jsonrpc-http-server.git#ee72e4778583daf901b5692468fc622f46abecb6"
source = "git+https://github.com/ethcore/jsonrpc-http-server.git#cd6d4cb37d672cc3057aecd0692876f9e85f3ba5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason for bump?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing jsonrpc_http_server::cors stuff (was private before)

@@ -1875,7 +1852,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "ws"
version = "0.5.2"
source = "git+https://github.com/ethcore/ws-rs.git?branch=mio-upstream-stable#609b21fdab96c8fffedec8699755ce3bea9454cb"
source = "git+https://github.com/ethcore/ws-rs.git?branch=mio-upstream-stable#e3d21c119350e753fdf4475b8cd88103b2280540"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason for bump?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing req.method() and adding some logic related to CONNECT handling (still unfinished though, but decided to split it into separate PR)


let handler = endpoint.and_then(|v| match v {
"apps" => Some(as_json(&self.api.list_apps())),
"ping" => Some(ping_response(&self.api.local_domain)),
"ping" => Some(ping_response()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming: response::ping()?

if env::var("RUST_LOG").is_ok() {
let mut builder = LogBuilder::new();
builder.parse(&env::var("RUST_LOG").unwrap());
builder.init().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_ok -> unwrap is better done with if let Ok(...).

what's the guarantee that the RUST_LOG flags are valid? edit: well, if it's only used in test code, the unwrap is alright :)

@rphmeier rphmeier added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 21, 2016
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Oct 21, 2016
@arkpar
Copy link
Collaborator

arkpar commented Oct 21, 2016

So does this mean that the user needs to expose 8180 instead of 8080 to access from external network now?

@jacogr
Copy link
Contributor

jacogr commented Oct 21, 2016

As in the current version, the secure apps are served via :8180, i.e. in 1.3.x it was the Signer in 1.4 is it the full built-in UI. (In both cases the apps are using the secure WS APIs).

All Dapps are still served from :8080 and is only allowed to utilise the RPC APIs via :8080/rpc/ - this includes apps installed into ~/.parity/dapps as well as the built-in dapps, i.e. Gavcoin, TokenReg, Registry, Basiccoin, GithubHint & the method signature registry.

In short - if the user (in the past) used the signer over the network via :8180, the same goes. Since we don't have stand-alone home & status apps, external network setup would need to cater for this config as well. If users use only our app, :8180 is enough, if they use dapps as well :8080 is also required.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants