-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implement standardized build pipeline based on existing repos #27
Conversation
3e02179
to
1cbae8f
Compare
|
||
# Move to working directory | ||
WORKDIR /opt/nebra-gatewayrs | ||
|
||
ARG SYSTEM_TIMEZONE | ||
ARG GATEWAY_RS_RELEASE | ||
ARG SYSTEM_TIMEZONE=Europe/London |
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.
other docker files don't set timezone. I think we can remove this to be consistent.
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 you get a warning if this isn't done. We should prob keep this and make that the standard.
# NOTE:: not sure we even need to do this. We should set the right environment or | ||
# get it from hm-pyhelper and it should be correct. We are doing this only to make | ||
# sure that the gateway runs even when the I2C device is not present. | ||
mapfile -t data < <( i2cdetect -y "${I2C_NUM}" ) |
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.
There is a separate ticket for making it pull variant information from hm-pyhelper. Region will probably still remain a env variable override. #24
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.
Looks good, small comments. Could you also fill in the PR description according to the template. Link to the corresponding issue, and double check all AC.
.devbots/needs-triage.yml
Outdated
@@ -0,0 +1,3 @@ | |||
enabled: true |
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.
@vpetersson we aren't using this anymore. Can we remove?
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 never got that one to fully work, so we can remove it.
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.
removed.
repo: helium/gateway-rs | ||
# keep bumping the excludes, right now we only have alpha to track. | ||
# later we will start tracking beta and finally release | ||
excludes: prerelease, draft |
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.
Does helium/gateway-rs have any prerelease and draft releases now? I don't see them?
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.
no right now they seem to have alpha only. I kept it as a reminder so that we can add alpha and beta to it when they start releasing those. Seemed like good to have.
ee3c647
to
409c974
Compare
if: env.UPDATED == 'true' | ||
uses: peter-evans/repository-dispatch@v1 | ||
with: | ||
token: ${{ secrets.MR_BUMP }} |
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.
Couldn't find right information about secrets. hm-miner was using secrets.MINER_UPDATE. is MR_BUMP right?
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.
@shawaj any difference in how to use those secrets?
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.
No difference, just that originally I called it miner update then changed to Mr bump in newer repos
start-gatewayrs.sh
Outdated
echo "Using ECC for public key." | ||
export GW_KEYPAIR="ecc://i2c-${I2C_NUM}:96&slot=0" | ||
else | ||
echo "gateway-rs deb package provided key /etc/helium_gateway/gateway_key.bin will be used." |
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.
Is this key randomly generated every build? If not, I think we should fail and exit here. Don't want to accidentally try to register a device with publicly available keys.
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 this is default location where gateway-rs will generate a random key at startup. This is mostly a devtool so that gateway doesn't fail in absence of ecc. I don't think we are supposed to hit this part in production environment.
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.
Yeah we need to have it use the ECC at all times for production. Maybe have an env variable that can override this for dev?
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.
added an environment variable.
409c974
to
e0cbd18
Compare
* clean and fix start-gateway.sh * introduce gen-region.sh * make sure github actions run successfuly
e0cbd18
to
08d5027
Compare
Why
How
References
issue #25