Adding Helper Tagets to multichain-testing Makefile
#10093
Conversation
| .PHONY: corepack-enable | ||
| corepack-enable: | ||
| corepack enable |
There was a problem hiding this comment.
make corepack-enable has no value over corepack enable.
In general, I think we should get away from all these .PHONY targets. A makefile is for making. If we just want an inventory of commands, that's better in a package.json "scripts" list or just the documentation.
There was a problem hiding this comment.
+1. And I don't think we should use the Makefile for anything JS related either.
Developers should also learn about corepack much earlier, from getting-started>installing yarn, and we shouldn't need to re-document it here.
| yarn test test/install-contracts.test.ts | ||
|
|
||
| .PHONY: all | ||
| all: setup install |
There was a problem hiding this comment.
this one seems worthwhile since it has setup and install dependencies. but I still think it would be better as a script in scripts because it's not idempotent. "Making" should be repeatable and have the same outcome.
There was a problem hiding this comment.
We have make start that does everything here except setup. The wait-for-pods command is also more robust than the sleeps here.
setup only needs to be called in two scenarios:
- First time user who doesn't have the needed dependencies or an
agshipkind cluster - User chose to stop things with
make stop cleaninstead of justmake stop, so we need to recreate a kind cluster
A potential harm of including setup in the start or an all script is that it will fail if the user hasn't called make clean. This would suggest that we should amend start to clean setup install...
I am not sure what my ultimate opinion is here. I see benefit in adding clean setup to the current all script, but also see it adding extra overhead for non-first-time users.
There was a problem hiding this comment.
Appreciate the contribution. As @turadg mentioned, I think we only need to land some of this:
-
include
setupin thestartscript or a newallscript -
Document
update clientflow when relayers get out of sync
| ############################################################################### | ||
|
|
||
| .PHONY: teardown | ||
| teardown: stop-forward stop clean delete |
There was a problem hiding this comment.
stop already calls stop-forward and delete, and clean calls stop. This would be simplified to teardown: clean which is redundant.
| .PHONY: corepack-enable | ||
| corepack-enable: | ||
| corepack enable |
There was a problem hiding this comment.
+1. And I don't think we should use the Makefile for anything JS related either.
Developers should also learn about corepack much earlier, from getting-started>installing yarn, and we shouldn't need to re-document it here.
| yarn test test/install-contracts.test.ts | ||
|
|
||
| .PHONY: all | ||
| all: setup install |
There was a problem hiding this comment.
We have make start that does everything here except setup. The wait-for-pods command is also more robust than the sleeps here.
setup only needs to be called in two scenarios:
- First time user who doesn't have the needed dependencies or an
agshipkind cluster - User chose to stop things with
make stop cleaninstead of justmake stop, so we need to recreate a kind cluster
A potential harm of including setup in the start or an all script is that it will fail if the user hasn't called make clean. This would suggest that we should amend start to clean setup install...
I am not sure what my ultimate opinion is here. I see benefit in adding clean setup to the current all script, but also see it adding extra overhead for non-first-time users.
|
|
||
| .PHONY: hermes-update | ||
| hermes-update: | ||
| kubectl exec -i hermes-agoric-osmosis-0 -c relayer -- hermes update client --host-chain agoriclocal --client 07-tendermint-1 |
There was a problem hiding this comment.
I like that we have this at our disposal and @Jovonni figured out a solution for expired clients. I would prefer to see us document the pain point/symptoms in an FAQ section of the README and we could document the kubectl command there. There are 3 different relayer nodes and 6 different clients that could go out of sync and this only covers one of the scenarios.
This FAQ item would also mention that this typically only happens if the environment has been running for a long time (several hours?) and another path is simply restarting the service.
|
Thanks for the comments - agreed with most of them. Will close this now! |
@Jovonni recommends devs to add these targets to
multichain-testingMakefile to run/testdapp-orchestration-basics. If we find them useful, it would be nicer to have them here to being with.Refs: Relevant section in README of orca-dapp.