-
Notifications
You must be signed in to change notification settings - Fork 7
Update Testnode for Nitro v3.5.2 #97
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
base: integration
Are you sure you want to change the base?
Conversation
|
|
||
| # Wait for the sequencer HTTP to be ready | ||
| echo "Waiting for sequencer HTTP to be ready..." | ||
| while ! curl -sf http://localhost:8547 > /dev/null; do |
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.
do we want some time limit on this or will this always eventually be ready?
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.
Good question, it should normally be ready at some point but nothing wrong in adding an extra check. I'm just not sure what should be the higher bound.
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.
it might not be necessary, i was just curious
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.
It won't be necessary for CI but I think for the sake of not keeping things running longer than they need to we could fail and exit the script after a certain number of iterations to impose a time limit. However, we can do it in another PR.
Something like this maybe.
| while ! curl -sf http://localhost:8547 > /dev/null; do | |
| max_iterations=12 # Multiplied by the sleep value in the loop to calculate how long the sequencer has to start. | |
| iterations=0 | |
| while ! curl -sf http://localhost:8547 > /dev/null; do | |
| echo "Sequencer HTTP is not ready yet. Retrying in 5 seconds..." | |
| sleep 5 | |
| if [$iterations -eq $max_iterations]; then | |
| fail "Could not connect to Sequencer HTTP server after $max_iterations attempts." | |
| fi | |
| ((iterations +=1)) | |
| done |
Where fail is some function like this from the migration test:
# This is a utility function for exiting with with error strings
fail(){
echo "$*" 1>&2; exit 1;
}
Sneh1999
left a comment
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.
LGTM! Thanks @jjeangal 🎉
lukeiannucci
left a comment
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.
nice! 🐐
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.
This lgtm for the update. We can improve the smoke test, but I think we can worry about it later. Also, thanks for the great PR description!
|
|
||
| # Wait for the sequencer HTTP to be ready | ||
| echo "Waiting for sequencer HTTP to be ready..." | ||
| while ! curl -sf http://localhost:8547 > /dev/null; do |
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.
It won't be necessary for CI but I think for the sake of not keeping things running longer than they need to we could fail and exit the script after a certain number of iterations to impose a time limit. However, we can do it in another PR.
Something like this maybe.
| while ! curl -sf http://localhost:8547 > /dev/null; do | |
| max_iterations=12 # Multiplied by the sleep value in the loop to calculate how long the sequencer has to start. | |
| iterations=0 | |
| while ! curl -sf http://localhost:8547 > /dev/null; do | |
| echo "Sequencer HTTP is not ready yet. Retrying in 5 seconds..." | |
| sleep 5 | |
| if [$iterations -eq $max_iterations]; then | |
| fail "Could not connect to Sequencer HTTP server after $max_iterations attempts." | |
| fi | |
| ((iterations +=1)) | |
| done |
Where fail is some function like this from the migration test:
# This is a utility function for exiting with with error strings
fail(){
echo "$*" 1>&2; exit 1;
}
Closes this and that Asana Task
This pull request includes several updates to test out Nitro version 3.5.2 and nitro contracts 2.1.3.
This PR:
release-goldendoodle)cff556bc886a4383a3936dce206ceef0b019fd62(v2.1.3)v2.1.3-alpha(tag)This PR does not:
Key places to review:
docker-compose.yaml: Changes to the Espresso node image configurationtest-node.bash: Updates to Nitro contracts version and branchsmoke-test.bash: New sequencer HTTP readiness check implementationHow to test this PR:
./smoke-test.bashNumber of confirmed staking nodes: 1