Skip to content
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

Pin the image versions #102

Merged
merged 3 commits into from
Mar 15, 2024
Merged

Pin the image versions #102

merged 3 commits into from
Mar 15, 2024

Conversation

ImJeremyHe
Copy link
Member

No description provided.

@@ -1,7 +1,7 @@
version: '3.9'
services:
orchestrator:
image: ghcr.io/espressosystems/espresso-sequencer/orchestrator:main

Choose a reason for hiding this comment

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

Wonder why we're using the arbmusl tag?

Choose a reason for hiding this comment

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

Ah so this is the musl image from the arb branch? Maybe call the branch arbitrum-integration to be a bit more descriptive?

Copy link
Member Author

@ImJeremyHe ImJeremyHe Mar 14, 2024

Choose a reason for hiding this comment

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

Oh. I checkouted a branch called arb in sequencer repo and run the static-build action manually. And it created this tag. I don't know if it is appropriate to do that. But I think it is convenient for us to track and update

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem. Let's update the branch name. Then the tag would be arbitrum-integrationmusl

Choose a reason for hiding this comment

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

We usually use the non-static docker images (it shouldn't matter here though) but it's probably better to be consistent.

If you want the branch to have docker images published when new commits are pushed to it I think you need to add the branch to the github actions triggers. For example here: https://github.com/EspressoSystems/espresso-sequencer/blob/3f5beae8e9f121987ecdacebb87be7ca5c0ab99d/.github/workflows/build.yml#L7

Or you can call the branch release-... then it will match an existing branch trigger.

Copy link
Member Author

@ImJeremyHe ImJeremyHe Mar 15, 2024

Choose a reason for hiding this comment

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

I think a better way for the integration is to publish the images manually. That means the test won't be broken by any code update. And if some one is going to publish the new version with this tag, and he will be responsible for checking all the tests.

@sveitser sveitser self-requested a review March 15, 2024 11:42
Copy link

@sveitser sveitser left a comment

Choose a reason for hiding this comment

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

LGTM but please see my comment about having published docker images from the sequencer repo.

@ImJeremyHe ImJeremyHe merged commit ef0772c into integration Mar 15, 2024
6 checks passed
@ImJeremyHe ImJeremyHe deleted the jh/pin branch March 15, 2024 13:47
zacshowa pushed a commit that referenced this pull request Nov 26, 2024
Post audit fixes for native token decimals feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants