Add ENV VAR FLTYE_ADMIN_ENDPOINT (#4948)
TL;DR
Add support for assigning admin endpoint by passing config via ENV vars
Type
- [ ] Bug Fix
- [x] Feature
- [ ] Plugin
Are all requirements met?
- [x] Code completed
- [x] Smoke tested
- [x] Unit tests added
- [ ] Code documentation added
- [ ] Any pending items have an associated Issue
Complete description
I found that the admin endpoint value is assigned by .yaml configure file via viper, however, the viper accessor function is written in flytestdlib and the repo is already set as read-only, so instead I added a function to override the config if env var is set, after err := configAccessor.UpdateConfig(context.TODO()) in root.go. Also, further need for customized env var could be added in this function as well.
for testing, please set the ENV var $FLTYE_ADMIN_ENDPOINT to the desired url, if ENV var is set, it would be used as the endpoint of admin instead the one specified in the config yaml file. e.g.
export FLTYE_ADMIN_ENDPOINT=localhost:30080
Tracking Issue
https://github.com/flyteorg/flyte/issues/4948
Follow-up issue
NA
Codecov Report
Attention: Patch coverage is 52.00000% with 12 lines in your changes are missing coverage. Please review.
Project coverage is 67.59%. Comparing base (
848ab01) to head (15debc2).
| Files | Patch % | Lines |
|---|---|---|
| cmd/config/env_var_reader.go | 59.09% | 6 Missing and 3 partials :warning: |
| cmd/root.go | 0.00% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #466 +/- ##
==========================================
- Coverage 67.65% 67.59% -0.06%
==========================================
Files 148 149 +1
Lines 6643 6668 +25
==========================================
+ Hits 4494 4507 +13
- Misses 1859 1868 +9
- Partials 290 293 +3
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 67.59% <52.00%> (-0.06%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
All fields that show up in cli are also settable through env vars. If it's --connection.endpoint them it'll be FLYTE_CONNECTION_ENDPOINT or something like that... You shouldn't need this PR IMO.
Hi @EngHabu, thanks for pointing this out! I also saw this v.viper.AutomaticEnv() in flytestdlib that I suppose it should be used to bind flags with env var, since the corresponding flag I was trying to assign is --admin.endpoint, I tried both FLYTE_ADMIN_ENDPOINT and ADMIN_ENDPOINT but didn't work.
I found this err := v.bindViperConfigsFromEnv(r) function in flytestdlib which I'm not so sure about, but I guess it might bound --admin.endpoint with ADMIN.ENDPOINT? which is not able to be set as an env var.