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

Run hApp-bundles during develpment with `hc run` #1939

Merged
merged 24 commits into from Dec 5, 2019
Merged

Conversation

@lucksus
Copy link
Member

lucksus commented Dec 3, 2019

PR summary

Currently hApp developers can only run very simple, one DNA hApps with hc run. And they even have to host their own local HTTP server for the UI.

Every hApp that is a bit more complex (multiple DNAs, bridges, etc.) can't be easily run during development. This lead devs to run the production conductor with a real conductor config. That in turn required them to couple the UI to this development setup for instance by injecting interface URLs/ports via ENVs. These patterns are not compatible with production deployments.

What hApp devs should do is start their hApp by writing a hApp-bundle as the hApps manifest. That conveys all information to setup any run-time to run the hApp (be it dev, production Holoscape or production Holo).

This PR adds the needed capability to hc to read hApp-bundles and start a development conductor accordingly.

hc run will now first look for a file bundle.toml in the current working directory and if it is present will read it and configure the internal conductor to implement that hApp bundle, i.e. add all the instances and UIs (via its static HTTP server) and run that. If there is no bundle.toml file present it will proceed as before.

hc run will not follow HTTP links to download DNAs or UIs and will also not unzip UI archives. It expects the URIs to be of type file: for DNAs and of new type dir: for the UIs. The latter has to point to a directory and will be served by the static web server of the conductor at the given port. It should be exactly that UI directory that will be packaged into a zip file when deploying the bundle.

Main benefit

This makes it possible to run complex hApp setups with multiple DNAs and bridges without having to write and maintain a conductor config. With that it enables a hApp development workflow that is centered around hApp-bundles from start and thus future-proof for deployment. It solves the problems discussed in #1600.

followups

Add functionality to hc for creating hApp bundles and updating them with the right DNA hash and UI directory path.

changelog

  • if this is a code change that effects some consumer (e.g. zome developers) of holochain core, then it has been added to our between-release changelog with the format
- summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)

documentation

@lucksus lucksus requested a review from thedavidmeister Dec 3, 2019
@lucksus lucksus marked this pull request as ready for review Dec 4, 2019
lucksus added 2 commits Dec 4, 2019
@lucksus lucksus added the NEEDS REVIEW label Dec 4, 2019
Copy link
Member

zippy left a comment

This looks pretty good, but I don't see any tests whatsoever, which worries me, because this is the kind of thing that folks will start to rely on. Also I thought that at one point we removed from the conductor the ability to do static serving, so I'm surprised that's still here. Are you sure that still works?

@philipbeadle

This comment has been minimized.

Copy link
Member

philipbeadle commented Dec 4, 2019

Looks good Nico except that as a web dev I really want to use the React tools to run the front end, so could we make it run an npm command for the web server step? That way it addresses Zippy's issue as well.

}

impl HappBundle {
pub fn id_references_are_consistent(&self) -> Result<(), String> {

This comment has been minimized.

Copy link
@willemolding

willemolding Dec 4, 2019

Contributor

This is great. It would be really easy to wasm-bindgen this integrity check code for holoscape so it isn't duped in Rust and Javascript

@lucksus

This comment has been minimized.

Copy link
Member Author

lucksus commented Dec 4, 2019

@zippy, the static web server works. I used the Community bundle for testing and had it running in my browser pointing to localhost. Agree that tests should be added. I don't fully understand how the npm step could help with that though, @philipbeadle?

@philipbeadle

This comment has been minimized.

Copy link
Member

philipbeadle commented Dec 5, 2019

Because I'd like to be able to make changes to my web code and see those changes. Rather than have to make a change, build, and rerun hcrun.

@thedavidmeister

This comment has been minimized.

Copy link
Contributor

thedavidmeister commented Dec 5, 2019

@philipbeadle @lucksus we can include npm in the wrapped hc in holonix if we need...

Copy link
Contributor

thedavidmeister left a comment

i do approve and am also concerned about the lack of tests around this and hc in general like @zippy is, as i'm seeing things like https://forum.holochain.org/t/nix-shell-does-not-have-latest-version-of-holochain/1450/5?u=thedavidmeister hc generate not working being reported in the forums

a lot of my time recently has been spent retroactively fixing hc issues after the binary is built and hits holonix then problems are picked up by the integration tests there

also i appreciate that adding tests might need to be a "next sprint" thing 🤷‍♂

@thedavidmeister thedavidmeister merged commit 5c146f6 into develop Dec 5, 2019
8 checks passed
8 checks passed
ci/circleci: app-spec-tests-sim1h Your tests passed on CircleCI!
Details
ci/circleci: app-spec-tests-sim2h Your tests passed on CircleCI!
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: cli-tests Your tests passed on CircleCI!
Details
ci/circleci: cluster-tests Your tests passed on CircleCI!
Details
ci/circleci: fmt Your tests passed on CircleCI!
Details
ci/circleci: stress-tests-sim2h Your tests passed on CircleCI!
Details
ci/circleci: wasm-conductor-tests Your tests passed on CircleCI!
Details
@lucksus

This comment has been minimized.

Copy link
Member Author

lucksus commented Dec 5, 2019

@philipbeadle, the static web server of the conductor does not cache files. If you change a file and reload the page you will get the new content without having to restart hc run. (just confirmed locally)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.