2.8.0: {[testenv]setenv} substitution broken #595

yogi-sagar · 2017-09-01T18:27:42Z

While installing tempest as part of devstack install, below commnd blows up with an error -
tox -r --notest -efull

Error -

2017-09-01 18:22:20.661 | ++ lib/tempest:install_tempest:612          :   pushd /opt/stack/new/tempest
2017-09-01 18:22:20.661 | ~/tempest ~/devstack
2017-09-01 18:22:20.662 | ++ lib/tempest:install_tempest:613          :   tox -r --notest -efull
2017-09-01 18:22:21.720 | Traceback (most recent call last):
2017-09-01 18:22:21.720 |   File "/usr/local/bin/tox", line 11, in <module>
2017-09-01 18:22:21.720 |     sys.exit(cmdline())
2017-09-01 18:22:21.720 |   File "/usr/local/lib/python2.7/dist-packages/tox/session.py", line 38, in main
2017-09-01 18:22:21.720 |     config = prepare(args)
2017-09-01 18:22:21.720 |   File "/usr/local/lib/python2.7/dist-packages/tox/session.py", line 26, in prepare
2017-09-01 18:22:21.720 |     config = parseconfig(args)
2017-09-01 18:22:21.720 |   File "/usr/local/lib/python2.7/dist-packages/tox/config.py", line 242, in parseconfig
2017-09-01 18:22:21.720 |     parseini(config, inipath)
2017-09-01 18:22:21.720 |   File "/usr/local/lib/python2.7/dist-packages/tox/config.py", line 796, in __init__
2017-09-01 18:22:21.721 |     replace=name in config.envlist)
2017-09-01 18:22:21.721 |   File "/usr/local/lib/python2.7/dist-packages/tox/config.py", line 827, in make_envconfig
2017-09-01 18:22:21.721 |     res = meth(env_attr.name, env_attr.default, replace=replace)
2017-09-01 18:22:21.721 |   File "/usr/local/lib/python2.7/dist-packages/tox/config.py", line 964, in getdict_setenv
2017-09-01 18:22:21.721 |     definitions = self._getdict(value, default=default, sep=sep)
2017-09-01 18:22:21.721 |   File "/usr/local/lib/python2.7/dist-packages/tox/config.py", line 975, in _getdict
2017-09-01 18:22:21.721 |     name, rest = line.split('=', 1)
2017-09-01 18:22:21.721 | ValueError: need more than 1 value to unpack
2017-09-01 18:22:21.733 | + lib/tempest:install_tempest:1            :   exit_trap

Logs link - http://logs.openstack.netapp.com/ci-logs/logs/65/499765/5/upstream-check/cinder-cDOT-FCP/3566c47/logs/devstacklog.txt.gz

The text was updated successfully, but these errors were encountered:

obestwalter · 2017-09-01T18:59:36Z

Thanks @yogi-sagar - I will have a look right away.

adamrothman · 2017-09-01T19:02:34Z

@obestwalter This broke my builds as well (looks like the same error):

$ make local-test
ORBIT_AWS__SERVICES__DYNAMODB__ENDPOINT_URL=http://192.168.99.100:8000 \
	ORBIT_SERVICES__NEO4J__HOST=192.168.99.100 \
	ORBIT_SERVICES__REDIS__HOST=192.168.99.100 \
	AWS_ACCESS_KEY_ID=[REDACTED] AWS_SECRET_ACCESS_KEY=[REDACTED] \
	tox --recreate
Traceback (most recent call last):
  File "/usr/local/bin/tox", line 11, in <module>
    sys.exit(cmdline())
  File "/usr/local/lib/python3.6/site-packages/tox/session.py", line 38, in main
    config = prepare(args)
  File "/usr/local/lib/python3.6/site-packages/tox/session.py", line 26, in prepare
    config = parseconfig(args)
  File "/usr/local/lib/python3.6/site-packages/tox/config.py", line 242, in parseconfig
    parseini(config, inipath)
  File "/usr/local/lib/python3.6/site-packages/tox/config.py", line 796, in __init__
    replace=name in config.envlist)
  File "/usr/local/lib/python3.6/site-packages/tox/config.py", line 827, in make_envconfig
    res = meth(env_attr.name, env_attr.default, replace=replace)
  File "/usr/local/lib/python3.6/site-packages/tox/config.py", line 964, in getdict_setenv
    definitions = self._getdict(value, default=default, sep=sep)
  File "/usr/local/lib/python3.6/site-packages/tox/config.py", line 975, in _getdict
    name, rest = line.split('=', 1)
ValueError: not enough values to unpack (expected 2, got 1)
make: *** [local-test] Error 1

adamrothman · 2017-09-01T19:11:12Z

Here's my tox.ini, if it helps:

[tox]
envlist = py36


[testenv]
deps =
    -rrequirements.txt
    flake8
    mypy
    pyflakes>=1.6.0
    pytest
    pytest-asyncio
passenv = ASYNC_TEST_TIMEOUT AWS_ACCESS_KEY_ID AWS_DEFAULT_REGION AWS_SECRET_ACCESS_KEY ORBIT_AWS__SERVICES__DYNAMODB__ENDPOINT_URL ORBIT_SERVICES__NEO4J__HOST ORBIT_SERVICES__REDIS__HOST
setenv =
    ORBIT_ENV = test
    ASYNC_TEST_TIMEOUT = 10
    AWS_DEFAULT_REGION = us-west-2
commands =
    flake8 orbit tests
    mypy --ignore-missing-imports orbit tests
    pytest {posargs} tests/unit
    pytest {posargs} tests/integration


[testenv:docker]
deps =
    flake8
    mypy
    pyflakes>=1.6.0
    pytest
    pytest-asyncio
sitepackages = True


[testenv:verbose]
setenv =
    {[testenv]setenv}
    ORBIT_AWS__SERVICES__DYNAMODB__ENDPOINT_URL = http://192.168.99.100:8000
    ORBIT_SERVICES__NEO4J__HOST = 192.168.99.100
    ORBIT_SERVICES__REDIS__HOST = 192.168.99.100
commands =
    pytest -s -vv {posargs}


[testenv:lint]
commands =
    flake8 orbit tests
    mypy --ignore-missing-imports orbit tests


[flake8]
ignore = E501

obestwalter · 2017-09-01T19:15:18Z

{[testenv]setenv} breaks with the new changes looks like we don't have a test for this substitution behaviour yet. I will create a reproducer and see if we can fix that with a quick bug fix soon. Please stick with 2.7 until then.

cboylan · 2017-09-01T19:16:58Z

#521 (diff) is the source of the bug. Basically if you run tox -efoo then config.envlist == ['foo'] but tox will still attempt to parse envs bar and baz and foobar. But none of those will get replacements done beacuse those names are not in ['foo'].

Project: openstack-infra/project-config 0bcbb286b03007d87c217703605a77969a2d1cfb Pin tox to 2.7.0 There is a fun new bug [0] in tox 2.8.0 that affects tempest. To avoid this crashing everything to a hault pin to 2.7.0 for now. [0] tox-dev/tox#595 Change-Id: I811fa4c71b63cfa39c3fd1018f9e3d3117ee2300

There is a fun new bug [0] in tox 2.8.0 that affects tempest. To avoid this crashing everything to a hault pin to 2.7.0 for now. [0] tox-dev/tox#595 Change-Id: I811fa4c71b63cfa39c3fd1018f9e3d3117ee2300

cboylan · 2017-09-01T19:43:58Z

Digging in more I see two potential "fixes" for the original issue #515. The first one is more of a workaround. Just set a default on all env vars (granted you may want to fail in some cases rather than relying on a default so maybe this isn't so great). The other is always replace internal tox key:value pairs and treat the replace flag as an indication of whether or not you should replace external key:value pairs. Or maybe just add another flag for external vs internal.

But I am not familiar with this code base so will defer to others.

obestwalter · 2017-09-01T19:55:06Z

minimal reproducer for this is:

[tox]
envlist = spam

[testenv]
setenv = DONTCARE = 0

[testenv:eggs]
setenv = {[testenv]setenv}

changelog and reproducer for #595

obestwalter · 2017-09-01T20:47:06Z

@cboylan - if you want to try for a fix, have a go - reproducer is in place - it's getting late here on this side of the planet, so I might have a look tomorrow :)

When reading in the config and processing dict content we were splitting on = assuming we would always have key = value pairs. However in the case where we don't want to replace because the testenv will not be used this can result in ValueErrors as substitution variables are not string key = value pairs. Avoid all this by only attempting to process dict key = values if we are replacing in the first place. Otherwise the content isn't needed and we just return the default value or empty dict. This should fix tox-dev#595

cboylan · 2017-09-01T22:37:58Z

This isn't a complete fix. The original tempest tox.ini now fails on missing boolean values when run against the commit above. I figured I'd throw something up before my day ends though.

obestwalter · 2017-09-02T08:59:33Z

Let's keep this open until the fix is out ...

obestwalter · 2017-09-02T17:46:09Z

I pushed a test package with the completed fix to devpi if somebody wants to test this:

pip install --index https://devpi.net/obestwalter/dev/+simple "tox==2.8.1.dev8+gda3a86a"

I tried already with tempest and that works again.

Project: openstack-infra/puppet-jenkins 5fc0bbbdb3d549bd0088e866180881b541da5646 Pin tox to 2.7.0 Tox 2.8.0 has broke us, clarkb has a patch upstream to fix. In the mean time, we can pin to 2.7.0. tox-dev/tox#595 Change-Id: I1079fb1bc6606a33b5b2517cca0c8eba2c12ce13 Signed-off-by: Paul Belanger <pabelanger@redhat.com>

Tox 2.8.0 has broke us, clarkb has a patch upstream to fix. In the mean time, we can pin to 2.7.0. tox-dev/tox#595 Change-Id: I1079fb1bc6606a33b5b2517cca0c8eba2c12ce13 Signed-off-by: Paul Belanger <pabelanger@redhat.com>

obestwalter · 2017-09-03T14:55:03Z

changed the implementation

pip install --index https://devpi.net/obestwalter/dev/+simple "tox==2.8.1.dev11+g64e9012"

obestwalter · 2017-09-03T19:26:13Z

Last dev version before release (I promise):

pip install --index https://devpi.net/obestwalter/dev/+simple "tox==2.8.1.dev13+g35e0dcc"

obestwalter · 2017-09-04T08:05:17Z

I think I will merge the fix and release a bugfix update today. If someone could review first it would be nice, but I am pretty confident that the fix does not make things worse ...

* #595 fix by updating implementation of #515 The way #515 was implemented did not play nicely with env var substitution. This might render the former additions completely or partially obsolete but I did not check this yet. First let's fix that bug. * fix flakes (cherry picked from commit e902f04) * #595 fix by updating implementation of #515 The way #515 was implemented did not play nicely with env var substitution. This might render the former additions completely or partially obsolete but I did not check this yet. First let's fix that bug. Instead of crashing early on testenv creation if the substitution the missing substitution keys are added to a list attached to TestenvConfig. This way it is not necessary anymore to provide everything needed to resolve all testenvs but instead only what is needed to resolve all testenvs that are part of this restrun. Implementation notes: The first implementation used a module level mapping from envname to missing substitutions, which was ugly. Instead the information is bubbled up as an exception where it can be handled the way it is needed in the contexts where the information can be either translated into a crashing error or attached to the object which should carry the information (in this case TestenvConfig). Atm a missing substitution in a testenv is not being left empty but filled with a special value 'TOX_MISSING_SUBSTITUTION', this is not exactly necessary, but I don't feel comfortable enough yet to leave it empty, this can help with debugging potential problems that might arise from this change. * #595 missed a bit Crash only if actually trying to run a testenv with missing substitutions * fix error on execution Instead of crashing the whole thing again - only later, do it the right way by setting venv.status with the error to be reported later. * #595 tests and last touches * catch MissingSubstition from all method calls for consistency * set status and stop execution in setupvenv if substutions are missing to prevent errors occuring there with missing substitutions

obestwalter · 2017-09-04T11:43:53Z

Fixed in 2.8.1

adamrothman · 2017-09-04T15:56:49Z

Thanks @obestwalter, I'll try 2.8.1 out tomorrow and let you know how it goes.

obestwalter added area:configuration bug:normal affects many people or has quite an impact labels Sep 1, 2017

obestwalter added a commit that referenced this issue Sep 1, 2017

Merge pull request #596 from Avira/master

a766e06

changelog and reproducer for #595

cboylan mentioned this issue Sep 1, 2017

Don't attempt to parse dicts if not replacing #597

Merged

3 tasks

obestwalter closed this as completed in #597 Sep 2, 2017

obestwalter reopened this Sep 2, 2017

obestwalter self-assigned this Sep 2, 2017

obestwalter mentioned this issue Sep 2, 2017

#595 fix by updating implementation of #515 #598

Closed

obestwalter mentioned this issue Sep 3, 2017

Fix substitution problems introduced in 2.8 #599

Merged

obestwalter changed the title ~~OpenStack - latest tox version broke OpenStack tempest~~ 2.8.0: {[testenv]setenv} substitution broken Sep 3, 2017

obestwalter closed this as completed in #599 Sep 4, 2017

This was referenced Sep 4, 2017

add comment stripping from config file (issue #332) #529

Closed

Introduce an additional configuration file format? #600

Closed

haikuginger mentioned this issue Sep 4, 2017

Use FQDN only for DNS and drop trailing dot for other operations urllib3/urllib3#1255

Merged

obestwalter added a commit that referenced this issue Sep 15, 2017

fix oversight in fixing #595: inherit new exception from Error (#635)

0091126

tox-dev locked and limited conversation to collaborators Jan 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2.8.0: {[testenv]setenv} substitution broken #595

2.8.0: {[testenv]setenv} substitution broken #595

yogi-sagar commented Sep 1, 2017

obestwalter commented Sep 1, 2017

adamrothman commented Sep 1, 2017 •

edited by The-Compiler

Loading

adamrothman commented Sep 1, 2017

obestwalter commented Sep 1, 2017 •

edited

Loading

cboylan commented Sep 1, 2017

cboylan commented Sep 1, 2017

obestwalter commented Sep 1, 2017

obestwalter commented Sep 1, 2017 •

edited

Loading

cboylan commented Sep 1, 2017

obestwalter commented Sep 2, 2017

obestwalter commented Sep 2, 2017 •

edited

Loading

obestwalter commented Sep 3, 2017 •

edited

Loading

obestwalter commented Sep 3, 2017

obestwalter commented Sep 4, 2017

obestwalter commented Sep 4, 2017

adamrothman commented Sep 4, 2017

2.8.0: {[testenv]setenv} substitution broken #595

2.8.0: {[testenv]setenv} substitution broken #595

Comments

yogi-sagar commented Sep 1, 2017

obestwalter commented Sep 1, 2017

adamrothman commented Sep 1, 2017 • edited by The-Compiler Loading

adamrothman commented Sep 1, 2017

obestwalter commented Sep 1, 2017 • edited Loading

cboylan commented Sep 1, 2017

cboylan commented Sep 1, 2017

obestwalter commented Sep 1, 2017

obestwalter commented Sep 1, 2017 • edited Loading

cboylan commented Sep 1, 2017

obestwalter commented Sep 2, 2017

obestwalter commented Sep 2, 2017 • edited Loading

obestwalter commented Sep 3, 2017 • edited Loading

obestwalter commented Sep 3, 2017

obestwalter commented Sep 4, 2017

obestwalter commented Sep 4, 2017

adamrothman commented Sep 4, 2017

adamrothman commented Sep 1, 2017 •

edited by The-Compiler

Loading

obestwalter commented Sep 1, 2017 •

edited

Loading

obestwalter commented Sep 1, 2017 •

edited

Loading

obestwalter commented Sep 2, 2017 •

edited

Loading

obestwalter commented Sep 3, 2017 •

edited

Loading