Fix setup for ext packages. #122
Conversation
Previously the wheels would contain the metadata but not the actual code.
Oberon00
requested review from
a-feld,
c24t,
carlosalberto,
lzchen,
reyang and
toumorokoshi
as code owners
| @@ -30,8 +30,8 @@ commands_pre = | |||
| test: pip install -e {toxinidir}/opentelemetry-api | |||
| test-sdk: pip install -e {toxinidir}/opentelemetry-sdk | |||
| ext: pip install -e {toxinidir}/opentelemetry-api | |||
| wsgi: pip install -e {toxinidir}/ext/opentelemetry-ext-wsgi | |||
| http-requests: pip install -e {toxinidir}/ext/opentelemetry-ext-http-requests | |||
| wsgi: pip install {toxinidir}/ext/opentelemetry-ext-wsgi | |||
There was a problem hiding this comment.
I think this is done to expose the packaging problem. With the -e flag enabled, the tests will pass regardless of what appears in the package since all tests are just pointing at the existing directory structure.
There was a problem hiding this comment.
makes sense. would be great to verify and comment as such, this type of configuration can get really hard to understand without some comments.
There was a problem hiding this comment.
Ln32 is using -e, what's the difference here?
ext: pip install -e {toxinidir}/opentelemetry-api
There was a problem hiding this comment.
One build without -e should be enough to detect the problem, was my thinking.
There was a problem hiding this comment.
Ah, I see this is a different package. You are right, we could remove the -e there too to increase coverage.
There was a problem hiding this comment.
I added a comment and removed the -e also from the other pip calls (but only in the test environments, lint keeps it).
|
I'm going ahead and merging this, I hope I addressed your comments properly, @reyang. |
Previously the wheels would contain the metadata but not the actual
code.