Skip to content
This repository was archived by the owner on Dec 9, 2025. It is now read-only.

Conversation

@deannagarcia
Copy link
Member

We recently saw a failure pass through where requirements.txt was updated in a way that was not compatible with some python versions. This updates the tests to directly use requirements.txt to download requirements to catch issues like this in the future.

@deannagarcia deannagarcia requested a review from haberman March 22, 2023 20:02
- uses: actions/upload-artifact@v3
with:
name: requirements
path: python/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Where is the source of truth for this file? Where does the data come from? A comment would be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean by that? The file is just being pulled out of the repository.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see. I thought maybe it was coming from a genrule().

Does it need to be uploaded at all then? Could we use a regular checkout action to obtain it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I only wanted requirements.txt to be available to us and not the rest of the repository (to ensure we aren't erroneously depending on another aspect of the repository).

Is it possible to do that with checkout?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. A comment to that effect would be helpful. Otherwise I would assume that any artifact you are uploading was produced by the build process. It would help to know the rationale for why a source was being uploaded as an artifact.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, added a comment!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants