Add Contrib Repo Tests workflow on Core PRs#1357
Add Contrib Repo Tests workflow on Core PRs#1357lzchen merged 11 commits intoopen-telemetry:masterfrom
Conversation
f61071d to
43439ca
Compare
NathanielRN
left a comment
There was a problem hiding this comment.
Just flagging some doubts on tox cache because I'm unfamiliar with it.
| with: | ||
| path: .tox | ||
| key: tox-cache-${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }} | ||
| key: tox-cache-${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-core |
There was a problem hiding this comment.
Not sure if this is needed...
There was a problem hiding this comment.
I think this is good, should keep the caches smaller
There was a problem hiding this comment.
Not too familiar with the syntax, what is this pointing to? The environment name?
There was a problem hiding this comment.
When I run tox locally, I see a .tox folder gets created, and I see packages being created there.
My guess is that this command caches packages that have the same conditions of python version/matrix/os/files_hash to not have to import the package into it's "tox" environment again. But that's just a guess.
| uses: actions/cache@v2 | ||
| with: | ||
| path: .tox | ||
| key: tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-core |
There was a problem hiding this comment.
Not sure if this is needed...
| uses: actions/cache@v2 | ||
| with: | ||
| path: .tox | ||
| key: tox-cache-${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-contrib |
There was a problem hiding this comment.
Not sure if this is needed...
| with: | ||
| path: .tox | ||
| key: tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }} | ||
| key: tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-contrib |
There was a problem hiding this comment.
Not sure if this is needed...
43439ca to
3eb2fc1
Compare
d1f811f to
bbf12c7
Compare
26b77cf to
859d9df
Compare
|
I see this issue happening in a fork: I thought this issue was not happening in the core repo but it seems like it is happening there as well: |
ocelotl
left a comment
There was a problem hiding this comment.
See an issue happening when running tox -e lint, explained in the conversation.
|
@ocelotl Thanks for the logs! I think you are seeing this problem because you did not clone the Contrib Repo using Please see my logs below, I tried to follow your commands and found that running |
ocelotl
left a comment
There was a problem hiding this comment.
Thanks for this work @NathanielRN, moving the non-core components out of the core repo is a massive task 💪
You probably already have thought about this, having releases of the instrumentation packages should save us from having to clone the contrib repo, right? Is it something we plan on doing?
|
@ocelotl Thanks for your review! 😄 Yes it's exactly like you said :) Once we're in GA and we have releases of all the packages, the Contrib repo packages will all be able to depend on them and install them from Pypi without any repo cloning. That way the progress of the Core Repo & Contrib Repos can be truly independent if we wish, yet we will still have tests to identify discrepancies. |
80559ba to
a783036
Compare
aabmass
left a comment
There was a problem hiding this comment.
One blocking comment, lmk what you think
| with: | ||
| path: .tox | ||
| key: tox-cache-${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }} | ||
| key: tox-cache-${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-core |
There was a problem hiding this comment.
I think this is good, should keep the caches smaller
6d9c6d2 to
bb3605b
Compare
2e8258b to
b833981
Compare
0f9890a to
528b8b0
Compare
528b8b0 to
dd7ba78
Compare
|
|
||
| ; opentelemetry-example-app | ||
| py3{5,6,7,8}-test-core-example-app | ||
| pypy3-test-core-example-app |
There was a problem hiding this comment.
Please note the removal of tests for the Example App on pypy3. The Example App uses grpcio which is not available on pypy packages.
Description
Add tests to check if PRs on the Core repo will break tests from the Contrib Repo.
To test against a particular Contrib Repo PR, change the env variable
CONTRIB_REPO_SHAto prove tests pass. Once they pass, PRs in Core and Contrib can be merged at the same time.This fixes:
tox -e linttox -e docsRelated to #1233
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
New tests added to make sure Core changes don't affect Contrib.
Checklist:
- [ ] Changelogs have been updated- [ ] Documentation has been updated