Skip to content

Dapp ropsten deployment#1441

Merged
nkuba merged 22 commits intomasterfrom
dapp-ropsten-deployment
Mar 13, 2020
Merged

Dapp ropsten deployment#1441
nkuba merged 22 commits intomasterfrom
dapp-ropsten-deployment

Conversation

@r-czajkowski
Copy link
Contributor

@r-czajkowski r-czajkowski commented Mar 10, 2020

Depends on: #1438
Closes: #1400

test build

New configs are deployed to keep-dev, available @ http://keep-dapp-token-dashboard.default.svc.cluster.local/ (behind the VPN)

@r-czajkowski r-czajkowski changed the base branch from master to tokens-page-grantee March 10, 2020 17:54
@@ -1 +1,2 @@
REACT_APP_ETH_NETWORK_WEB_SOCKET_ADDRESS=ws://eth-tx-node.default.svc.cluster.local:8546 No newline at end of file
REACT_APP_ETH_NETWORK_WEB_SOCKET_ADDRESS=ws://eth-tx-node.default.svc.cluster.local:8546
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this filename match the truffle network name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep exactly. Good catch. The main idea is to keep consistent with the keep-dev and keep-test. What do you think about this? Or maybe we should create a new environment variable dedicated to the dapp?

@@ -1 +1,2 @@
REACT_APP_ETH_NETWORK_WEB_SOCKET_ADDRESS=ws://eth-tx-node.default.svc.cluster.local:8546 No newline at end of file
REACT_APP_ETH_NETWORK_WEB_SOCKET_ADDRESS=ws://eth-tx-node.default.svc.cluster.local:8546
REACT_APP_ETHERSCAN_DEFAULT_URL='https://etherscan.io/address/' No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't play nicely against the private network.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine that it's here, just wanted to call it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly leave a comment.

Copy link
Contributor

@sthompson22 sthompson22 left a comment

Choose a reason for hiding this comment

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

We're going to need additional configuration on the Kube side since we'll be exposing the Ropsten instance to the internet. Specifically an Ingress and Service with type NodePort. Do you want to do that, or should I pick that up in this PR?

@sthompson22
Copy link
Contributor

Another thought - should we rename staking-dapp to token-dapp everywhere? I think staking is too narrow?

@r-czajkowski
Copy link
Contributor Author

Another thought - should we rename staking-dapp to token-dapp everywhere? I think staking is too narrow?

Agree. I thought about it. I'll update also in the circleci config.

@r-czajkowski
Copy link
Contributor Author

@sthompson22, token-dashboard-dapp will be ok? Or would you prefer a shortened version token-dapp?

@sthompson22
Copy link
Contributor

token-dashboard-dapp will be ok? Or would you prefer a shortened version token-dapp

Thanks for asking, right now it's keep-dapp-staking everywhere. I think keep-dapp-token-dashboard sounds better.

@r-czajkowski r-czajkowski marked this pull request as ready for review March 12, 2020 10:42
@r-czajkowski r-czajkowski changed the base branch from tokens-page-grantee to master March 12, 2020 13:33
Evnets subscirpiton works well without the websocket provider
.env files are not needed anymore. The default ehterscan address can be
returned from the custom funcion.
@sthompson22
Copy link
Contributor

Thanks for going through these updates. I'm going to take it for a spin in keep-dev now.

@sthompson22
Copy link
Contributor

To give a bit of color on my naming recommendation in the last comment keep-dapp-token-dashboard

This allows us to be consistent around the naming pattern prefix project-thing-function, easier to parse files, and allows a label scheme that matches such as:

app: keep-dapp
type: keep-token-dashboard

Not super relevant now but say we create some new dapp for Keep, we call it "operations" in this example. Using the format above that would give us:

keep-dapp-operations

app: keep-dapp
type: operations

Benefit here is that it binds deployed applications that are of the same breed, sort of like labeling "frontend" and "backend", in a more specific way. Anywho I'm not firm on this, just wanted to explain my thought process on the original suggestion.

Just fine with it as-is

@r-czajkowski
Copy link
Contributor Author

ah right Sloan. I missed it. I'll update

r-czajkowski and others added 2 commits March 12, 2020 15:34
We've removed all of the .env files, so there's nothing to copy anymore.
@sthompson22
Copy link
Contributor

I had to do some surgery to get a keep-dev deployment without contract migrations happening in the same build (e.g. if contracts you want to use are already migrated). That was done #1454 here.

To get this out to Ropsten, if it's not coupled with a contract deployment we'll need to do something similar.

@sthompson22
Copy link
Contributor

@nkuba @r-czajkowski Thanks again ya'll for getting this put together. I've run through a deployment to keep-dev, and as a result pushed one small change here. Kuba if you want to do the honors of merging?

export const OPERATOR_CONTRACT_NAME_EVENTS = 'eventKeepRandomBeaconOperatorContract'

export const ETHERSCAN_DEFAULT_URL = 'https://etherscan.io/address/'
export const ETHERSCAN_DEFAULT_URL = 'https://ropsten.etherscan.io/address/'
Copy link
Member

Choose a reason for hiding this comment

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

Please remember about handling it per environment in a follow-up PR.

@nkuba nkuba merged commit 6e99790 into master Mar 13, 2020
@nkuba nkuba deleted the dapp-ropsten-deployment branch March 13, 2020 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ropsten KEEP token dashboard deployment

4 participants