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

Optimize environment variable behavior #333

Merged
merged 18 commits into from
Jun 14, 2022
Merged

Optimize environment variable behavior #333

merged 18 commits into from
Jun 14, 2022

Conversation

xiaoyongzhu
Copy link
Member

@xiaoyongzhu xiaoyongzhu commented Jun 8, 2022

Currently it's a bit confusing for end users and there are multiple asks on what are all the user configurable environment variables (and the expected behaviors).

Also, we only read a portion of the env variables in key vault, but we should actually support reading all the env variables in KV.

This PR solves the above issues, by having a clear logic on how to get the env variables in the following order:

  1. If the key is set in the envrionment variable, Feathr will use the value of that environment variable
  2. If it's not set in the environment, then a value is retrieved from from the feathr_config.yaml file with the same config key.
  3. If it's not available in the feathr_config.yaml file, Feathr will try to reterive the value from a key vault service. Currently only Azure Key Vault is supported.

Also add docs describing all the env variables.

@xiaoyongzhu xiaoyongzhu linked an issue Jun 9, 2022 that may be closed by this pull request
@blrchen blrchen added the safe to test Tag to execute build pipeline for a PR from forked repo label Jun 12, 2022
@xiaoyongzhu xiaoyongzhu merged commit 41e7496 into main Jun 14, 2022
@xiaoyongzhu xiaoyongzhu deleted the xiaoyzhu/key_vault branch June 14, 2022 04:23
bozhonghu pushed a commit that referenced this pull request Jun 15, 2022
* main:
  Fixing purview test issues and improve performance (#350)
  [feathr] Add product_recommendation advanced sample (#348)
  obejectId query cmd update (#360)
  add license, release, docs, python api ref badges with shields img (#357)
  quick fix the 404 not found in read me link (#355)
  Python SQL Registry (#311)
  enable JWT token param in frontend API calls (#337)
  Optimize environment variable behavior (#333)
  Adding better warning message to let user know that config file is missing and they need to set env parameters. (#347)
  Feature Monitoring (#330)
  Windoze/211 maven submission (#334)
  Windoze/211 maven submission (#334)
  Windoze/211 maven submission (#334)
  Fix Synapse quickstart link (#346)
  Show feature details when click feature in lineage graph (#339)
  Update pull_request_push_test.yml
  Update UI README for how to create overrides for local development (#335)
  Update databricks quick start experience (#217)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test Tag to execute build pipeline for a PR from forked repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize environment variable behavior
4 participants