这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@MaxJa4
Copy link
Contributor

@MaxJa4 MaxJa4 commented Aug 27, 2023

Closes #253

Changes

All environment variables can now be used with the _FILE convention.
This means, that e.g. WEBDAV_PASSWORD can also be supplied via docker secrets by using WEBDAV_PASSWORD_FILE instead.

Note: All value (non-file) variants are preferred over their _FILE variant.

Todo

  • README update
  • Tests

@MaxJa4
Copy link
Contributor Author

MaxJa4 commented Aug 28, 2023

@m90 used mostly your code from #253 for the lookup function and pointed the envconfig module to our fork for now. Will open a PR to the original repo soon. I'd wait a little, maybe it gets merged rather soon, so we can change offen/envconfig back to how it was before in the go.mod/imports.

I also experimented with the --timeout option for docker compose, as DVB always gets forcibly killed by compose after the default 10s timeout. Reason seems to be, that crond does not properly implement the SIGINT signal, so it doesn't react to the container stop. Since containers are recreated and stopped a lot in the tests, this should speed them up a bit without causing any issues.
I used 3 seconds here, since containers usually stop almost immediately and they get recreated anyway. That reduced the CI time from ~16:30 min. to ~13:00 min. Since we're adding more and more tests, this can maybe help us a little to not someday need 30 mins just für CI tests. This change shouldn't be added here though, it's just a test that I'll remove again later. Maybe you want to include that in your #261 PR about the docker/testing setup :)

Edit: The swarm test was untouched, but it failed with this: Failed to remove network s5l3rwmsjddcq4amhbwn1kcei: Error response from daemon: rpc error: code = FailedPrecondition desc = network s5l3rwmsjddcq4amhbwn1kcei is in use by task f0c5ecbzh2or5prhuzuv885waFailed to remove some resources from stack: test_stack. I added a 1s delay after test_stack to hopefully not let this sporadic issue come up again.

@m90
Copy link
Member

m90 commented Aug 29, 2023

This looks like a great addition, thanks.

I'd wait a little, maybe it gets merged rather soon, so we can change offen/envconfig back to how it was before in the go.mod/imports.

Ok, just let me know when you're done waiting. Or not.

I also experimented with the --timeout option for docker compose,

Good idea, I'll add these to #261

MaxJa4 added 2 commits August 29, 2023 11:17
This reverts commit ab50b21, reversing
changes made to 0282514.

Revert "Test compose timeout option"

This reverts commit 0282514.
@MaxJa4
Copy link
Contributor Author

MaxJa4 commented Aug 29, 2023

Created the PR to the source repo: kelseyhightower/envconfig#210
Merged our changes into offen/envconfig@master and created a new v1.5.0 version for Go to target.

@MaxJa4 MaxJa4 marked this pull request as ready for review August 29, 2023 17:51
@MaxJa4
Copy link
Contributor Author

MaxJa4 commented Aug 30, 2023

Done from my side :)

Copy link
Member

@m90 m90 left a comment

Choose a reason for hiding this comment

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

💯

@m90 m90 merged commit 43c4961 into offen:main Aug 30, 2023
@MaxJa4 MaxJa4 deleted the envconfig branch August 30, 2023 18:17
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.

Support _FILE based configuration values for all config keys

2 participants