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

Fix broken overrides #287

Merged
merged 16 commits into from
Feb 12, 2023
Merged

Fix broken overrides #287

merged 16 commits into from
Feb 12, 2023

Conversation

afanasev
Copy link
Contributor

@afanasev afanasev commented Jul 7, 2022

Trying to fix #271 and similiar issues once again. Added some tests and make them pass. Did NOT check 4 inlude tests since they're not working on Win.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 95.295% when pulling 17a739c on afanasev:master into be660de on chimpler:master.

@darthbear
Copy link
Member

Thank you @afanasev for your PR!

@darthbear darthbear merged commit 7ab4d34 into chimpler:master Feb 12, 2023
darthbear pushed a commit that referenced this pull request Feb 12, 2023
* Trying to fix #271
---------

Co-authored-by: Alexandr Afanasiev <afanasev@devexperts.com>
@carolguo-dd
Copy link
Contributor

carolguo-dd commented Mar 3, 2023

Seems like this fix has introduced another problem. we set the the POD_NAMESPACE env variable, but not the K8s_NAMESPACE variable and have below input file

{
    k8s {
        namespace = ${POD_NAMESPACE}
        namespace = ${?K8S_NAMESPACE}

    }
    host= test-${k8s.namespace}
}

this used to work in 0.3.59 version, but now it breaks with

  File "/Users/carol.guo/.pyenv/versions/3.9.7/lib/python3.9/site-packages/pyhocon/config_parser.py", line 695, in resolve_substitutions
    raise ConfigSubstitutionException("Cannot resolve {variables}. Check for cycles.".format(
pyhocon.exceptions.ConfigSubstitutionException: Cannot resolve ${k8s.namespace}: (line: 7, col: 21). Check for cycles.

@fsonntag
Copy link

This change also broke some of our configs, see here:

#320

@darthbear Is it possible to revert this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong Order in Substitution Resolution Leads to Faulty Configuration
5 participants