Skip to content

Conversation

@Wauplin
Copy link
Collaborator

@Wauplin Wauplin commented Jun 13, 2024

Follow-up PR after #8417.

When github.ref is a tag (e.g. refs/tags/v0.29.0), the logic to determine PATH_IN_REPO (e.g. v0.29.0) is incorrect. See example where files have been hosted at the root of the folder. This PR fixes this. Sorry about that, I did not realize how github action inject values in the bash command. This time I tested it properly and it should work correctly.

EDIT: in parallel I also opened a PR to clean the mess in the dataset repo: https://huggingface.co/datasets/diffusers/community-pipelines-mirror/discussions/2

EDIT 2:: I also triggered a manual deployment to correctly push v0.29.0 (see CI) . Files have correctly been uploaded to v0.29.0/ folder: https://huggingface.co/datasets/diffusers/community-pipelines-mirror/tree/main/v0.29.0

@Wauplin Wauplin requested a review from sayakpaul June 13, 2024 07:53
# e.g. refs/tags/v0.28.1 -> v0.28.1
echo "CHECKOUT_REF=${{ github.ref }}" >> $GITHUB_ENV
echo "PATH_IN_REPO=${${{ github.ref }}#refs/tags/}" >> $GITHUB_ENV
echo "PATH_IN_REPO=$(echo ${{ github.ref }} | sed 's/^refs\/tags\///')" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

Tricky one. Thanks for taking care of this.

(side note: having an automated reporting mechanism would have promptly alerted us about this; so all the reasons for me to work on it soon)

Copy link
Collaborator Author

@Wauplin Wauplin Jun 13, 2024

Choose a reason for hiding this comment

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

Definitely!

Even more subtle, the github action passed ( ✔️ ) but its behavior was wrong. When working on a reporting mechanism, it would be good to report values from PATH_IN_REPO/CHECKOUT_REF to slack as well.

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Thanks!

@Wauplin Wauplin merged commit 06ee907 into main Jun 13, 2024
sayakpaul pushed a commit that referenced this pull request Dec 23, 2024
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.

3 participants