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

Light miner: Remove dbus container #409

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Conversation

pritamghanghas
Copy link
Contributor

@pritamghanghas pritamghanghas commented Mar 27, 2022

  • remove dbus container

Issue

How

Screenshots

References

Checklist

  • Tests added
  • Cleaned up commit history (rebase!)
  • Documentation added
  • Thought about variable and method names

@pritamghanghas pritamghanghas requested review from a team as code owners March 27, 2022 12:53
README.md Show resolved Hide resolved
@@ -48,15 +45,12 @@ services:
- {{I2C_DEVICE}}:{{I2C_DEVICE}}
restart: on-failure
environment:
- DBUS_SYSTEM_BUS_ADDRESS=unix:path=/session/dbus/session_bus_socket
- FIRMWARE_VERSION={{FIRMWARE_VERSION}}
- I2C_DEVICE={{I2C_DEVICE}}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this i2c device env variable?

Copy link
Contributor Author

@pritamghanghas pritamghanghas Mar 27, 2022

Choose a reason for hiding this comment

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

to detect ecc in the current startup script. Will be removed when we convert startup to python and start using variant information. NebraLtd/hm-gatewayrs#24

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok fair enough

settings.ini Outdated
@@ -1,9 +1,9 @@
[versions]
FIRMWARE_VERSION=2022.03.07.0-2
Copy link
Member

Choose a reason for hiding this comment

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

This should relate to the actual firmware version from gateway-rs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is auto bump action for it. but will update it manually for the PR.

Copy link
Member

Choose a reason for hiding this comment

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

If this is the case, should the auto bump have not done this? Is it actually working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this is the case, should the auto bump have not done this? Is it actually working?

there hasn't been a new release of gateway-rs since it was activated for hm-gatewayrs

Copy link
Member

Choose a reason for hiding this comment

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

But it should be looking at the FIRMWARE_VERSION here to see if it's up to date?

So clearly isn't working

Copy link
Member

@shawaj shawaj Mar 27, 2022

Choose a reason for hiding this comment

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

Ah I think I know why.... Scheduled workflows have to be in the master branch to work...

- DBUS_ADDRESS=unix:path=/session/dbus/session_bus_socket
- FIRMWARE_VERSION={{FIRMWARE_VERSION}}
- FIRMWARE_SHORT_HASH={{ENV.FIRMWARE_SHORT_HASH}}

volumes:
miner-storage:
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need miner-storage volume?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I had removed it forgot to stage the change.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, we probably need to leave this in for cases where people are using a data-only hotspot and want the file based private key to be persistent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checked all container again, hm-diag uses it for permanent storage.

Copy link
Member

Choose a reason for hiding this comment

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

Yes and hm-gatewayrs should also use it for persistent key storage for file based key

Ref NebraLtd/hm-gatewayrs#28

templates/docker-compose.template Show resolved Hide resolved
@pritamghanghas pritamghanghas force-pushed the remove_dbus branch 4 times, most recently from be65525 to 0c64839 Compare March 28, 2022 14:25
settings.ini Outdated Show resolved Hide resolved
@shawaj shawaj mentioned this pull request Mar 29, 2022
4 tasks
Copy link
Member

@shawaj shawaj left a comment

Choose a reason for hiding this comment

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

Still need to fix the comments about the workflows

@pritamghanghas
Copy link
Contributor Author

Still need to fix the comments about the workflows

@shawaj Which one, scheduled workflow one?

@shawaj
Copy link
Member

shawaj commented Mar 29, 2022

Still need to fix the comments about the workflows

@shawaj Which one, scheduled workflow one?

Yes this one:
#409 (comment)

@shawaj
Copy link
Member

shawaj commented Mar 29, 2022

Basically I think you will need to do another PR with new workflows to the master branch or the auto-updates won't work

Copy link
Member

@shawaj shawaj left a comment

Choose a reason for hiding this comment

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

As discussed on slack with @pritamghanghas just create new issue for the workflow (if not already there) and all good 👍

@pritamghanghas pritamghanghas merged commit d6e5f80 into light-hotspot Mar 29, 2022
@pritamghanghas pritamghanghas deleted the remove_dbus branch March 29, 2022 14:21
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.

4 participants