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

update dockerfile and default config #108

Merged
merged 15 commits into from
May 17, 2024
Merged

update dockerfile and default config #108

merged 15 commits into from
May 17, 2024

Conversation

piccione99
Copy link
Contributor

@piccione99 piccione99 commented Apr 23, 2024

  1. updated default config so we don't need a config file for running docker
  2. updated readme instructions
  3. added an example config file for reference
  4. couldn't get entrypoint.sh to work so currently not using it

@piccione99 piccione99 self-assigned this Apr 23, 2024
@piccione99 piccione99 marked this pull request as draft April 23, 2024 21:10
Dockerfile Outdated Show resolved Hide resolved
@piccione99 piccione99 changed the title temporary update to docker update dockerfile and default config May 16, 2024
@piccione99 piccione99 marked this pull request as ready for review May 16, 2024 15:19
Copy link
Contributor

@denopink denopink left a comment

Choose a reason for hiding this comment

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

the idea makes sense since it's a pattern we already put into practice for our services, i.e.: the service should be able to start with the default config.

However, do we want that to be true for xmidt-agent since it's running on a cpe? Not sure if that's a desired pattern for software running on cpe.

Approving this, but I think we shouldn't merged until @schmidtw chimes in.

@schmidtw
Copy link
Member

Yes, in general we want the app to basically work right out of the box. We also want it to be secure out of the box, too. I will review this later; there are a few parts being built into the configuration that I think we should do via environment variables (the device_id and other identity items being at the top of this list).

@piccione99 piccione99 merged commit dfbb42f into main May 17, 2024
14 checks passed
@piccione99 piccione99 deleted the docker branch May 17, 2024 17:46
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