Skip to content

chore: Added docs section on env vars in yaml#2792

Merged
feast-ci-bot merged 3 commits intofeast-dev:masterfrom
creativedutchmen:docs/yaml-env-replacement
Jun 15, 2022
Merged

chore: Added docs section on env vars in yaml#2792
feast-ci-bot merged 3 commits intofeast-dev:masterfrom
creativedutchmen:docs/yaml-env-replacement

Conversation

@creativedutchmen
Copy link
Copy Markdown
Contributor

Because it was unclear to me you could do this, and there was another question in Slack, I've added a section to the docs to explain you can do this:

project: my_project
registry: data/registry.db
provider: local
online_store:
    type: redis
    connection_string: ${REDIS_CONNECTION_STRING}

@creativedutchmen
Copy link
Copy Markdown
Contributor Author

I just noticed a previous commit (that I thought was already merged) has found its way into this PR. Please let me know if I should create a new PR with only the latest commit. Thanks.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 14, 2022

Codecov Report

Merging #2792 (aea5b0e) into master (0d195c4) will decrease coverage by 0.14%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2792      +/-   ##
==========================================
- Coverage   80.59%   80.44%   -0.15%     
==========================================
  Files         173      173              
  Lines       15006    15254     +248     
==========================================
+ Hits        12094    12271     +177     
- Misses       2912     2983      +71     
Flag Coverage Δ
integrationtests 70.25% <100.00%> (-0.79%) ⬇️
unittests 59.70% <100.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...stores/contrib/spark_offline_store/spark_source.py 61.15% <100.00%> (+0.56%) ⬆️
sdk/python/feast/infra/registry_stores/sql.py 70.38% <0.00%> (-7.46%) ⬇️
sdk/python/tests/utils/online_read_write_test.py 93.54% <0.00%> (-6.46%) ⬇️
.../integration/online_store/test_online_retrieval.py 96.84% <0.00%> (-3.16%) ⬇️
sdk/python/feast/stream_feature_view.py 88.03% <0.00%> (-2.88%) ⬇️
sdk/python/feast/field.py 91.89% <0.00%> (-2.40%) ⬇️
...ion/registration/test_stream_feature_view_apply.py 94.11% <0.00%> (-2.32%) ⬇️
sdk/python/feast/registry.py 82.71% <0.00%> (-0.99%) ⬇️
...ython/feast/embedded_go/online_features_service.py 89.14% <0.00%> (-0.78%) ⬇️
...k/python/feast/infra/offline_stores/file_source.py 88.07% <0.00%> (-0.72%) ⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d195c4...aea5b0e. Read the comment docs.

@achals
Copy link
Copy Markdown
Member

achals commented Jun 14, 2022

Thanks for the contribution @creativedutchmen ! I think the previous commit is okay to pull in since there's no merge conflicts.

@achals
Copy link
Copy Markdown
Member

achals commented Jun 14, 2022

/ok-to-test

Copy link
Copy Markdown
Member

@achals achals left a comment

Choose a reason for hiding this comment

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

/lgtm

@achals achals changed the title chore: added docs section on env vars in yaml chore: Added docs section on env vars in yaml Jun 14, 2022
@achals
Copy link
Copy Markdown
Member

achals commented Jun 14, 2022

Actually @creativedutchmen you'll need to signoff your commit - you should be able to follow the instructions at https://github.com/feast-dev/feast/pull/2792/checks?check_run_id=6883873559

Signed-off-by: Huib Keemink <huib.keemink@jedlix.com>
Signed-off-by: Huib Keemink <huib.keemink@jedlix.com>
@creativedutchmen creativedutchmen force-pushed the docs/yaml-env-replacement branch from 3c9bfef to 2ba4e2e Compare June 15, 2022 06:36
@feast-ci-bot feast-ci-bot removed the lgtm label Jun 15, 2022
@creativedutchmen
Copy link
Copy Markdown
Contributor Author

I just learned it's possible to set a default like ${ENV_VAR:"default"} - will update to add that.

Signed-off-by: Huib Keemink <huib.keemink@jedlix.com>
Copy link
Copy Markdown
Member

@achals achals left a comment

Choose a reason for hiding this comment

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

/lgtm

@feast-ci-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achals, creativedutchmen

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit bdecb10 into feast-dev:master Jun 15, 2022
@mkhalaf2
Copy link
Copy Markdown

mkhalaf2 commented Aug 7, 2023

creativedutchmen
@creativedutchmen
I've tried to set default - but it doesn't work anymore - to my understanding load_repo_config uses os.path.expandvars and the latter does not support default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants