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

Update push-test.yml #591

Merged
merged 4 commits into from
Oct 23, 2024
Merged

Update push-test.yml #591

merged 4 commits into from
Oct 23, 2024

Conversation

daxpryce
Copy link
Contributor

When the upload-artifact action was updated to v4, the behavior of it changes such that you cannot create a unified "artifact" by having it add files to an existing artifact name. Instead, they all require a unique artifact name to be uploaded.

This should fix the current workflow build errors.

When the upload-artifact action was updated to v4, the behavior of it changes such that you cannot create a unified "artifact" by having it add files to an existing artifact name. Instead, they all require a unique artifact name to be uploaded.

This should fix the current workflow build errors.
@daxpryce daxpryce requested a review from gopalrs October 15, 2024 21:25
@daxpryce daxpryce marked this pull request as draft October 15, 2024 21:38
@daxpryce
Copy link
Contributor Author

The artifact woes are more extensive than I thought - I will address them all in a single PR, not just one I made through the github file editor 😆

@gopalrs
Copy link
Contributor

gopalrs commented Oct 16, 2024

Thanks Dax, I didn't realize how complicated this change was :)

@daxpryce
Copy link
Contributor Author

Yeah, this is why I didn't upgrade it before, I think the only problem we'll have is the python wheel part since that'll be across like N different artifacts, but it's not insurmountable - just a lot of places to adjust. I'll try to get this together today :)

@daxpryce daxpryce marked this pull request as ready for review October 16, 2024 21:31
@daxpryce
Copy link
Contributor Author

This is still failing on one of the dynamic tests but it's getting surprising exit codes and isn't really in scope for this PR. Someone more competent in the windows-diskann-dynamic workflow space should probably check to see why sometimes we get random exit codes of 127 and such though. Seems like we have a bug somewhere?

@daxpryce
Copy link
Contributor Author

daxpryce commented Oct 16, 2024 via email

@gopalrs
Copy link
Contributor

gopalrs commented Oct 19, 2024

I will look into the other test failures, Dax. Thanks for fixing the upload issues.

@gopalrs gopalrs merged commit 364a565 into main Oct 23, 2024
38 of 39 checks passed
daxpryce added a commit that referenced this pull request Oct 23, 2024
* Update push-test.yml

When the upload-artifact action was updated to v4, the behavior of it changes such that you cannot create a unified "artifact" by having it add files to an existing artifact name. Instead, they all require a unique artifact name to be uploaded.

This should fix the current workflow build errors.

* Trying to fix the workflow to handle the name collision problem that starts in upload-artifact@v4

* Trying to get the cibuildwheel tool to work with the changes to AlmaLinux8 (why the heck is this docker image still shipping with an old gpg key anyway)

* Dynamic and Dynamic Labels were both trying to write to the  artifact
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