-
Notifications
You must be signed in to change notification settings - Fork 24
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
Handle case for branch rebase/force push #109
Merged
+28
−40
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
34dc617
Handle case for branch rebase/force push
neel-astro 2c836bc
update tests to latest runtime version
neel-astro 0e5da65
fix yq installation
neel-astro c6856a9
temp add branch to e2e tests
neel-astro 819d4f4
drop manual workflow add approval step
neel-astro 2694bfe
remove duplicate code
neel-astro 4823a4b
handle force push
neel-astro d727b89
fix force push case
neel-astro 1ed259c
fix PR cases
neel-astro ed360e5
fix typo
neel-astro e4dd5ea
fix logs statement
neel-astro b49f2b9
fix git mocks
neel-astro d24383a
update runtime version to 12.7.1
neel-astro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -319,8 +319,8 @@ runs: | |
GITHUB_EVENT_BEFORE=${{ github.event.before }} | ||
GITHUB_EVENT_AFTER=${{ github.event.after }} | ||
if [[ "$GITHUB_EVENT_BEFORE" == "0000000000000000000000000000000000000000" || -z $GITHUB_EVENT_BEFORE && -z $GITHUB_EVENT_AFTER ]]; then | ||
DBT_DEPLOY=true | ||
files=() | ||
DBT_DEPLOY=true | ||
files=() | ||
else | ||
files=$(git diff --name-only $GITHUB_EVENT_BEFORE $GITHUB_EVENT_AFTER) | ||
echo "files changed: $files" | ||
|
@@ -362,23 +362,26 @@ runs: | |
cd ${{ inputs.root-folder }} | ||
fi | ||
|
||
branch=$(echo "${GITHUB_REF#refs/heads/}") | ||
echo "Branch pushed to: $branch" | ||
git fetch origin $branch | ||
|
||
DAGS_ONLY_DEPLOY=false | ||
SKIP_IMAGE_OR_DAGS_DEPLOY=true | ||
SKIP_IMAGE_OR_DAGS_DEPLOY=false | ||
files=() | ||
|
||
# case when the triggered event is a manual workflow dispatch or a new branch or tag creation, we would need to deploy the image because we cannot determine that it does not need to be deployed | ||
GITHUB_EVENT_BEFORE=${{ github.event.before }} | ||
GITHUB_EVENT_AFTER=${{ github.event.after }} | ||
# case when the triggered event is a manual workflow dispatch or a new branch or tag creation, we would need to deploy the image because we cannot determine that it does not need to be deployed | ||
if [[ "$GITHUB_EVENT_BEFORE" == "0000000000000000000000000000000000000000" || -z $GITHUB_EVENT_BEFORE && -z $GITHUB_EVENT_AFTER ]]; then | ||
DAGS_ONLY_DEPLOY=false | ||
SKIP_IMAGE_OR_DAGS_DEPLOY=false | ||
files=() | ||
echo "Manual workflow dispatch or a new branch or tag creation, hence missing github event before and/or after commit hash" | ||
else | ||
files=$(git diff --name-only $GITHUB_EVENT_BEFORE $GITHUB_EVENT_AFTER) | ||
echo "files changed: $files" | ||
echo "event that triggered the workflow: $GITHUB_REF" | ||
branch=$(echo "${GITHUB_REF#refs/heads/}") | ||
git fetch origin $branch | ||
if ! git cat-file -e "${{ github.event.before }}" 2>/dev/null; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in case of force push, we would get a commit hash, which won't exist in the git history due to force push |
||
echo "Commit ${{ github.event.before }} does not exist, falling back to image deploy." | ||
else | ||
SKIP_IMAGE_OR_DAGS_DEPLOY=true | ||
files=$(git diff --name-only ${{ github.event.before }} ${{ github.event.after }}) | ||
echo "files changed: $files" | ||
fi | ||
fi | ||
|
||
for file in $files; do | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
FROM quay.io/astronomer/astro-runtime:12.5.0 | ||
FROM quay.io/astronomer/astro-runtime:12.7.1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove references to manual workflow dispatch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manual workflow dispatch will also have the GitHub event before and after as empty/null right? Or am I missing anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we were removing manual workflow dispatch as a way of invoking this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, only fixing the case for rebase/force push, while ensuring manual workflow dispatch works as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merging this PR, but let me know if we still need to adjust the log statement, happy to handle it.