Ct 10485/env var none

[kentkr]

Resolves #10485

Problem

none is not acceptable as a default for env_var. It throws an error saying the env var is not defined. This behavior is unexpected and different than var

Solution

I used a sentinel approach: the default is an empty object. If that object gets overwritten with None it's ok. If not it throws the env var missing exception.

Checklist

• I have read the contributing guide and understand what's expected of me.
• I have run this code in development, and it appears to resolve the stated issue.
• This PR includes tests, or tests are not required or relevant for this PR.
• This PR has no interface changes (e.g., macros, CLI, logs, JSON artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX.
• This PR includes type annotations for new and modified functions.

[codecov]

Codecov Report

Attention: Patch coverage is 91.83673% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.86%. Comparing base (98fddcf) to head (92085c9).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10629      +/-   ##
==========================================
- Coverage   88.89%   88.86%   -0.04%     
==========================================
  Files         180      180              
  Lines       22779    22780       +1     
==========================================
- Hits        20250    20244       -6     
- Misses       2529     2536       +7     

Flag	Coverage Δ
integration	86.11% <91.83%> (-0.04%)	⬇️
unit	62.36% <42.85%> (+0.01%)	⬆️

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

Components	Coverage Δ
Unit Tests	62.36% <42.85%> (+0.01%)	⬆️
Integration Tests	86.11% <91.83%> (-0.04%)	⬇️

[kentkr]

@dbeatty10 Just checking in to see if this approach aligns with what we need. If there's anything we should discuss further for this feature?

[dbeatty10]

Hey @kentkr!

We haven't yet had a chance to assess #10485 to determine if we'd want to do it or not.

It's going to take a close examination because it looks like it was intentionally coded to have its current behavior here and here.

We're unlikely to prioritize looking at this more closely any time soon. But it's going to be super handy to have your PR as something tangible to consider 🤩

I'm sorry for any churn this has created for you. Here's the "secret decoder ring" for the GitHub labels we use for issues:

• triage -- we haven't decided yet!
• good_first_issue -- shouldn't be too hard, and we'd accept a contribution from a community member
• help_wanted -- probably quite tricky, and we'd accept a contribution from a community member
• none of the above -- we're planning on doing this ourselves

The best way to avoid churn for community PRs is when they are for good_first_issue or help_wanted issues. You are totally free to raise PRs even if they don't have those labels, but there's a much greater chance that review could be delayed or we'll close as "not planned" for some reason.

[kentkr]

Thanks @dbeatty10! I'll keep an eye out for the community tags next time.

I have a few more questions and comments but I'll add them to the original issue.