Making start-docker Contract-Agnostic
#93
Conversation
amessbee
changed the title
start-docker Contract-Agnostic --WIP--
make_ports_public.sh
Outdated
| @@ -0,0 +1,19 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Why is there code related to codespaces here? That should be in a different PR to make sure that we have a shorter PR to review here.
amessbee
force-pushed
the
ms/creating-contract-agnostic-docker
branch
from
ec15741 to
1b21a98
Compare
amessbee
changed the title
start-docker Contract-Agnostic --WIP--start-docker Contract-Agnostic
|
Asking users to install |
|
I am removing myself from the review. I think @Jovonni or @mujahidkay are best suited to review and help make progress in this area. |
amessbee
force-pushed
the
ms/creating-contract-agnostic-docker
branch
from
d984d0c to
0e73e6a
Compare
|
I feel this is at a reasonable shape now and we can invite reviews. Instead of the complex multi-step approach I started with above, I have now settled for a much simpler idea that disturbs least amount of artifacts to achieve the same IMO. To summarize, we
@dckc WDYT? |
| # Check if container exists but is stopped | ||
| if [ "$(docker ps -aq -f status=exited -f name=agdc)" ]; then | ||
| echo "Found stopped container 'agdc'. Removing it before starting a new one..." | ||
| docker rm agdc |
There was a problem hiding this comment.
Removing a container is irreversible, right? So if somebody happened to have a container named agdc running when trying out our stuff, this would remove it, yes? That doesn't seem fail-safe; it seems to violate the principle of least surprise.
| : ${DAPP_ED_CERT_PATH:="$(pwd)/../dapp-ed-cert"} | ||
| : ${DAPP_CHAIN_TIMER_PATH:="$(pwd)/../dapp-chain-timer"} | ||
| : ${SECOND_INVITE_PATH:="$(pwd)/../dapp-second-invite"} | ||
| : ${DAPP_OFFER_UP_PATH:="$(pwd)/../dapp-offer-up"} | ||
| : ${DAPP_AGORIC_BASICS_PATH:="$(pwd)/../dapp-agoric-basics"} |
There was a problem hiding this comment.
Why should dapp-offer-up know about these other dapps?
There was a problem hiding this comment.
Does this work if somebody clones dapp-offer-up but uses something like demo as the directory name?
Does yarn create @agoric/dapp demo (from getting started) work?
| # bring back chain process to foreground | ||
| wait | ||
| # Start new container with the chain startup commands | ||
| docker run -d \ |
There was a problem hiding this comment.
The docker-compose.yml syntax sure looked more straightforward.
What would you think of refactoring it to something like...
agd_image=...
linux_only=...
ports=...
...
docker run -d --name agdc $linux_only $ports $env $volumes $agd_image $start_agd
|
Do you plan to squash all the commits into 1? |
This is a twin of a PR in dapp-agoric-basics repo.
Goal of this PR is to (maybe partially) close #92. Currently running of
agoriclocalchain and deploying and starting contract are intertwined in this dapp. This is somewhat unnatural as chain should not depend on the files of any specific contract - apart from the ones that designated essential for its running.Further, running a chain independent of a specific contract will allow testing of multiple dapp simultaneously running and interacting with each other which may unlock several scenarios that a developer may want to test for.
As per discussion with @toliaqat, here is a plan:
run-chain.sh) that necessary tasks for running of chain independently.yarn start:contractshould handle most of the heavy work including:ws-offer-uphere)copy tools needed for building, i.e.,Makefilecopy scripts and tools needed for required for makinginstall-bundle,submit-proposal,voteetc. calls.create contract bundles on host machine, and then copy them to the workspace in docker container.build-proposal,install-bundle,submit-proposal,voteetc. calls needed to deploy and run the contract using scripts as before.update relevant docs. In particular, add guides to installagoricandagdCLI.Link to IBIS document.