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

Use remote.origin.url on airflow integration test #939

Merged
merged 3 commits into from
Feb 10, 2021

Conversation

wslulciuc
Copy link
Member

@wslulciuc wslulciuc commented Feb 10, 2021

This PR (attempts) to fix the failed integration tests for forked PRs. The main issue is that, for forks, the commit is not associated with the marquez repo, see pull/894, which causes the git checkout cmd to fail when installing a lib from a git url, see the failing CI job 656

We now dynamically build the url to point to remote.origin.url and using the rev for that repo.

@wslulciuc wslulciuc added review Ready for review and removed in progress labels Feb 10, 2021
@wslulciuc wslulciuc marked this pull request as ready for review February 10, 2021 19:37
@@ -20,8 +20,10 @@ set -e
project_root=$(git rev-parse --show-toplevel)
cd "${project_root}"/integrations/airflow/tests/integration

REV=$(git rev-parse HEAD)
MARQUEZ_AIRFLOW_LIB_WITH_REV="git+git://github.com/MarquezProject/marquez.git@${REV}#egg=marquez_airflow&subdirectory=integrations/airflow"
GIT_URL=$(git config --get remote.origin.url \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, we have to modify the link to use https, see pypa/pip#7554

@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #939 (e91bc49) into main (77d1404) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #939   +/-   ##
=========================================
  Coverage     72.53%   72.53%           
  Complexity      830      830           
=========================================
  Files           167      167           
  Lines          3780     3780           
  Branches        360      360           
=========================================
  Hits           2742     2742           
  Misses          568      568           
  Partials        470      470           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77d1404...e91bc49. Read the comment docs.

@wslulciuc wslulciuc merged commit 1c3484a into main Feb 10, 2021
@wslulciuc wslulciuc deleted the bug/airflow-integration-test-on-fork branch February 10, 2021 20:02
@wslulciuc wslulciuc removed the review Ready for review label Mar 23, 2021
mobuchowski pushed a commit that referenced this pull request Jul 27, 2021
* Use remote.origin.url on airflow integration test

Signed-off-by: wslulciuc <[email protected]>

* Fix git url for requirement.txt

Signed-off-by: wslulciuc <[email protected]>
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.

2 participants