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

Get ROCK image right tag considering the latest merged rock files #217

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

amandahla
Copy link
Collaborator

Applicable spec:

Overview

What happened in Synapse:

24 Nov: I submitted a WIP PR that changes the ROCK images canonical/synapse-operator#111
27 Nov: Publish to Edge workflow sucessfully ran with merged changes (update jsonschema)
27 Nov: I refreshed Synapse charm in production to apply the fix regarding nginx pebble ready event (23 Nov) + this last one regarding jsonschema
For my huge surprise: Synapse image is running the latest Synapse version as was changed in the WIP PR

Rationale

Make publish charm get the latest image from the latest merged PR and not the latest uploaded.

Workflow Changes

Checklist

  • The contributing guide was applied
  • The PR is tagged with appropriate label (urgent, trivial, complex)

@amandahla amandahla added trivial bug Something isn't working labels Nov 27, 2023
@amandahla amandahla requested a review from a team as a code owner November 27, 2023 20:16
arturo-seijas
arturo-seijas previously approved these changes Nov 28, 2023
Copy link
Collaborator

@arturo-seijas arturo-seijas left a comment

Choose a reason for hiding this comment

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

LGTM :) Thanks!

jdkandersson
jdkandersson previously approved these changes Nov 29, 2023
arturo-seijas
arturo-seijas previously approved these changes Nov 29, 2023
@amandahla amandahla dismissed stale reviews from arturo-seijas and jdkandersson via 54f82d3 November 29, 2023 14:36
arturo-seijas
arturo-seijas previously approved these changes Nov 30, 2023
@weiiwang01
Copy link
Collaborator

The problem of using head_sha is that we are using the squash merge, which causes the commit in main branch is different with the commit in the pull request.

For example, the commit in main branch is canonical/synapse-operator@7defdb4, but the corresponding commit in pull request is canonical/synapse-operator@1e13c04.

Unfortunately, tests inside this repository don't reflect that situation.

@weiiwang01
Copy link
Collaborator

The problem of using head_sha is that we are using the squash merge, which causes the commit in main branch is different with the commit in the pull request.

For example, the commit in main branch is canonical/synapse-operator@7defdb4, but the corresponding commit in pull request is canonical/synapse-operator@1e13c04.

Unfortunately, tests inside this repository don't reflect that situation.

I think something like this should work:

# Get commit info
TREE_SHA=$(gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/${{ github.repository }}/commits/${GITHUB_SHA} \
   --jq '.commit.tree.sha')
# Get workflow from this specific tree id
RUN_ID=$(gh api \
  --method GET \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/${{ github.repository }}/actions/runs \
  -f path=.github/workflows/integration_test.yaml \
  -f status=completed \
  -f event=pull_request \
  --jq "[
          .workflow_runs[]
          | select(.path == \".github/workflows/integration_test.yaml\")
          | select(.head_commit.tree_id == \"$TREE_SHA\")
        ] | max_by(.updated_at) | .id")

@amandahla
Copy link
Collaborator Author

The problem of using head_sha is that we are using the squash merge, which causes the commit in main branch is different with the commit in the pull request.
For example, the commit in main branch is canonical/synapse-operator@7defdb4, but the corresponding commit in pull request is canonical/synapse-operator@1e13c04.
Unfortunately, tests inside this repository don't reflect that situation.

I think something like this should work:

# Get commit info
TREE_SHA=$(gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/${{ github.repository }}/commits/${GITHUB_SHA} \
   --jq '.commit.tree.sha')
# Get workflow from this specific tree id
RUN_ID=$(gh api \
  --method GET \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/${{ github.repository }}/actions/runs \
  -f path=.github/workflows/integration_test.yaml \
  -f status=completed \
  -f event=pull_request \
  --jq "[
          .workflow_runs[]
          | select(.path == \".github/workflows/integration_test.yaml\")
          | select(.head_commit.tree_id == \"$TREE_SHA\")
        ] | max_by(.updated_at) | .id")

Thanks!!

Copy link
Contributor

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM!

@amandahla amandahla merged commit b9737af into main Dec 1, 2023
62 checks passed
@amandahla amandahla deleted the fix-publish-edge branch December 1, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants