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.

Sign up for GitHub

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
merged 13 commits into from
Feb 21, 2025
+28 −40
Merged

Handle case for branch rebase/force push #109

Show file tree
Hide file tree
Changes from 12 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 Feb 5, 2025
2c836bc
update tests to latest runtime version
neel-astro Feb 5, 2025
0e5da65
fix yq installation
neel-astro Feb 5, 2025
c6856a9
temp add branch to e2e tests
neel-astro Feb 5, 2025
819d4f4
drop manual workflow add approval step
neel-astro Feb 5, 2025
2694bfe
remove duplicate code
neel-astro Feb 5, 2025
4823a4b
handle force push
neel-astro Feb 5, 2025
d727b89
fix force push case
neel-astro Feb 5, 2025
1ed259c
fix PR cases
neel-astro Feb 5, 2025
ed360e5
fix typo
neel-astro Feb 5, 2025
e4dd5ea
fix logs statement
neel-astro Feb 5, 2025
b49f2b9
fix git mocks
neel-astro Feb 5, 2025
d24383a
update runtime version to 12.7.1
neel-astro Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 3 additions & 24 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,6 @@ name: E2E Tests for Astro Deploy Action

on:
push:
branches:
- main
workflow_dispatch:
inputs:
workspace_id:
description: "Workspace ID"
required: false
default: ""
org_id:
description: "Organization ID"
required: false
default: ""
astronomer_host:
description: "Astronomer Host"
required: false
default: ""
token:
description: "API Token"
required: false
default: ""

env:
ASTRO_API_TOKEN: ${{ secrets.ASTRO_API_TOKEN }}
Expand All @@ -30,6 +10,7 @@ jobs:
redact-inputs:
name: Redact Inputs
runs-on: ubuntu-latest
environment: e2e-test
steps:
- name: Checkout code
uses: actions/checkout@v4
Expand Down Expand Up @@ -79,9 +60,7 @@ jobs:
- name: Install dependencies
run: |

sudo add-apt-repository ppa:rmescandon/yq
sudo apt update
sudo apt install yq -y
wget https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 -O /usr/bin/yq && chmod +x /usr/bin/yq

curl -sSL https://install.astronomer.io | sudo bash -s
astro context switch ${{ steps.get-astro-env-info.outputs.astronomer_host }}
Expand Down Expand Up @@ -814,7 +793,7 @@ jobs:
with:
is_wake_on_deploy: ${{ matrix.deployment_id == needs.create-test-deployments.outputs.HIBERNATE_DEPLOYMENT_ID }}
dag_tarball_version_before: ""
image_version_before: "12.5.0"
image_version_before: "12.6.0"
hibernation_spec_before: '{"schedules":[{"isEnabled":true,"hibernateAtCron":"0 * * * *","wakeAtCron":"1 * * * *"},{"isEnabled":true,"hibernateAtCron":"1 * * * *","wakeAtCron":"0 * * * *"}]}'
dag_tarball_version_after: ${{ steps.get-deployment-after-create-preview.outputs.desired_dag_tarball_version }}
image_version_after: ${{ steps.get-deployment-after-create-preview.outputs.desired_image_version }}
Expand Down
29 changes: 16 additions & 13 deletions action.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Copy link
Collaborator

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?

Copy link
Collaborator Author

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?

Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
Expand Down
2 changes: 1 addition & 1 deletion e2e-setup/astro-project/Dockerfile
View file
Open in desktop
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.6.0
2 changes: 1 addition & 1 deletion e2e-setup/deployment-templates/deployment-hibernate.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ deployment:
configuration:
name: deploy-action-hibernate-e2e
description: ""
runtime_version: 12.5.0
runtime_version: 12.6.0
dag_deploy_enabled: true
ci_cd_enforcement: false
scheduler_size: SMALL
Expand Down
2 changes: 1 addition & 1 deletion e2e-setup/deployment-templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ deployment:
configuration:
name: deploy-action-non-dev-e2e
description: ""
runtime_version: 12.5.0
runtime_version: 12.6.0
dag_deploy_enabled: true
ci_cd_enforcement: false
scheduler_size: SMALL
Expand Down
2 changes: 2 additions & 0 deletions e2e-setup/mocks/dag-deploy-git.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ if [[ "$1" == "diff" ]]; then
echo "e2e-setup/astro-project/dags/exampledag.py"
elif [[ "$1" == "fetch" ]]; then
echo "Handling git fetch, doing nothing"
elif [[ "$1" == "cat-file" ]]; then
echo "Handling git cat-file, doing nothing"
else
echo "Error: git mock script isn't configured to handle $1" >&2
exit 1
Expand Down
2 changes: 2 additions & 0 deletions e2e-setup/mocks/image-deploy-git.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ if [[ "$1" == "diff" ]]; then
echo "e2e-setup/astro-project/Dockerfile"
elif [[ "$1" == "fetch" ]]; then
echo "Handling git fetch, doing nothing"
elif [[ "$1" == "cat-file" ]]; then
echo "Handling git cat-file, doing nothing"
else
echo "Error: git mock script isn't configured to handle $1" >&2
exit 1
Expand Down
2 changes: 2 additions & 0 deletions e2e-setup/mocks/no-deploy-git.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ if [[ "$1" == "diff" ]]; then
echo "README.md"
elif [[ "$1" == "fetch" ]]; then
echo "Handling git fetch, doing nothing"
elif [[ "$1" == "cat-file" ]]; then
echo "Handling git cat-file, doing nothing"
else
echo "Error: git mock script isn't configured to handle $1" >&2
exit 1
Expand Down