You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/03/09 20:29:24 UTC

[GitHub] [airflow] XD-DENG commented on pull request #14380: Prevent mixed case env vars from crashing a worker

XD-DENG commented on pull request #14380:
URL: https://github.com/apache/airflow/pull/14380#issuecomment-794413478


   Thanks @haard for the PR. I understand the only functional difference here is to avoid crash if there is "_mixed case env vars (starting with AIRFLOW__ but then not all uppercase)_". From that perspective, I agree this PR does its job and LGTM.
   
   What we may want to discuss a bit further are different options:
   - Option A: allow lowercases, and help users convert to upper case if necessary. This may not make sense given environment variables are case sensitive in Unix. But of course there may be different opinions.
   - Option B: don't allow lowercase, and choose to ignore (what you are doing)
   - Option C: don't allow lowercase, and reject explicitly. Maybe not making sense, because users may purposely want `AIRFLOW__CORE__lowercasekey` for whatever reason.
   
   So overall, I agree with your direction, i.e. option B.
   
   @ashb , may need your input here. Need your approval to unblock anyway :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org