-
Notifications
You must be signed in to change notification settings - Fork 51
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
Generator v3 #88
Generator v3 #88
Conversation
frontend: (process.env.VERSION_FRONTEND || 'v0.1.3') | ||
relayer: (process.env.VERSION_RELAYER || 'v0.1.4'), | ||
stats: (process.env.VERSION_STATS || 'v0.1.4'), | ||
frontend: (process.env.VERSION_FRONTEND || 'v0.1.4') |
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.
we should put default as latest
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 will bump it to the latest stable, everything latest doesn't guarantee to work
for our testing we can easy change to all latest in the initial config.
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.
After thinking some more this case will make a difference when the user redeploys. If they run 'docker pull' command (maybe accidentally) with the latest tag they will auto update their components. The consideration is if we want this behavior or static version. I think it's risky maybe bump version can have breaking changes, forking etc.
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 point. So better we remove this default version as well.
My point here is, if we keep updating our version. We need to come here to change again and again. Need to find a way to avoid this.
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 think it's ok, we are already the provider of stable versions. We have put the stable info somewhere. If not here, then have to find a way to pull that info. This can reduce 1 step of user manual task.
ENV IMAGE_NAME=${IMAGE_NAME} | ||
RUN echo SUBNET_BRANCH=${SUBNET_BRANCH} | ||
RUN echo IMAGE_NAME=${IMAGE_NAME} | ||
|
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.
why we need these?
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.
and what's IMAGE_NAME for
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.
generator clones the subnet repo for puppeth, so this is the way to record the subnet's branch/commit within this image.
the IMAGE_NAME is for the generated command to self reference its version, ex. the generator-v3 tag instead of latest.
docker run --env-file generated/common.env \ -v $(pwd)/generated/:/app/generated/ \ --entrypoint 'bash' xinfinorg/subnet-generator:generator-v3 ./deploy_csc.sh
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.
the args are injected at build-time, by github actions
|
||
|
||
|
||
|
||
FROM node:18.15 |
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.
suggest to use node alpine image to minimize the image
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 tried that but it cannot execute the puppeth binary
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.
ok
remaining to bump final components versions.