-
Notifications
You must be signed in to change notification settings - Fork 5
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
Check for Blocks before Contract Start and Adding Format Script #108
Conversation
aecdd7c
to
e8a1e17
Compare
e8a1e17
to
e5904f6
Compare
.gitignore
Outdated
@@ -39,3 +39,4 @@ videos | |||
|
|||
# contract tx | |||
contract/,tx.json | |||
contract/start-offer-up-plan.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drive-by? put it in a separate commit, perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :) - sure, will do.
contract/scripts/wait-for-chain.sh
Outdated
local required_patterns=3 # Number of block patterns we want to see | ||
|
||
while IFS= read -r line; do | ||
echo "$line" # Show the log output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider showing just 1 dot (.
) instead
if [[ $line =~ "block-manager: block "[0-9]+" commit" ]]; then | ||
((count++)) | ||
if [ $count -ge $required_patterns ]; then | ||
return 0 # Success |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps show the whole line at this point; i.e. give evidence in the form of a timestamp and block number that things are running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so rather than 150 lines of agd output in the ci log we would see...
Run yarn start:contract
Waiting for blockchain to start........(~150 dots)...
agd-1 | 2025-01-08T08:20:28.543Z block-manager: block 1227 commit
Blockchain is running and producing blocks...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like the dots all on the same line.
I suppose it's not critical, though.
Co-authored-by: Dan Connolly <[email protected]>
15c610a
to
60b9b18
Compare
yay! much nicer DX!
-- ci log |
echo "Blockchain is running and producing blocks..." | ||
exit 0 | ||
else | ||
echo "Failed to detect blockchain activity\n Run yarn start:docker to start the blockchain first!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops... I should have checked this part; it doesn't seem to work for me. If I do yarn start:contract
before yarn start:docker
, I get:
$ yarn start:contract
Waiting for blockchain to start...
............
agd-1 | 2024-12-13T14:50:03.351Z block-manager: block 44932 commit
Blockchain is running and producing blocks...
service "agd" is not running
maybe that won't affect 1st time users
perhaps it's because I have stuff in my docker logs from an earlier run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work fine for me:
Waiting for blockchain to start...
Failed to detect blockchain activity\n Run yarn start:docker to start the blockchain first!
Although newline character is not treated as such due to missing -e
I suppose. If I am going create a PR just for -e
, I am going treat myself with red color added to the above line too.
potentially closes: #27