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

Handle case for branch rebase/force push #109

Merged
merged 13 commits into from
Feb 21, 2025
Merged

Conversation

neel-astro
Copy link
Collaborator

@neel-astro neel-astro commented Feb 5, 2025

Changes:

  • Fix the case when force push would fail the deploy action around file diff computation
  • Upgrade runtime versions in our tests
  • Fix failing yq installation in the e2e test suite
  • Dropped the failing manual workflow trigger for the e2e test suite, rather now the suite gets triggered on each commit push on all branches, but we rather have an approval step to ensure that we don't end up creating too many resources on Astro.

Testing:

  • The e2e tests are happy
  • Manually validated the force push and rebase scenario to ensure that they are not running into issues
Screenshot 2025-02-05 at 2 58 52 PM Screenshot 2025-02-05 at 1 53 59 PM Screenshot 2025-02-05 at 1 57 07 PM

Relates to #95

@neel-astro neel-astro marked this pull request as ready for review February 5, 2025 09:42
@neel-astro neel-astro requested a review from a team as a code owner February 5, 2025 09:42
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

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.

@neel-astro neel-astro merged commit ef3b6be into main Feb 21, 2025
31 checks passed
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