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

Add setup.py to the Cache Keys for the GitHub Actions Workflow #78

Merged
merged 3 commits into from
Jul 22, 2021

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Jun 7, 2021

🗣 Description

This adds setup.py to the files hashed to create the cache keys used in our GitHub Actions workflow. I am intentionally not using **/setup.py so only the setup.py in the root directory of the repository (which should be the base path for GITHUB_WORKSPACE when running an Action) is used. This prevents unexpected behavior if a package contains a setup.py.

💭 Motivation and context

Since we direct requirements for our Python projects to be stored in setup.py we should invalidate a cache based on changes to that file. The file generally will not change much outside of requirements changes once a project is past it's initial implementation.

🧪 Testing

I confirmed that in the GHA run the cache key is updated to include a hash for setup.py.

📷 Screenshots (if appropriate)

✅ Checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

This adds a hash of `setup.py` to the cache keys used in the GitHub Actions
workflow for Python projects.
@mcdonnnj mcdonnnj added the improvement This issue or pull request will add or improve functionality, maintainability, or ease of use label Jun 7, 2021
@mcdonnnj mcdonnnj self-assigned this Jun 7, 2021
@mcdonnnj mcdonnnj requested a review from dav3r as a code owner June 7, 2021 12:24
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

Good improvement, though I'd recommend adding a comment explaining why setup.py is not **/setup.py.

The other cache keys for our GHA jobs are in the format '**/<filename>' so that
any file with that name is used in the repository. However, for Python packages
they may have a 'setup.py' as part of their internal codebase that does not
impact environment requirements. As a result we only want to use the 'setup.py'
that is in the root of the repository and is used to install the package.

Co-authored-by: dav3r <david.redmin@trio.dhs.gov>
@mcdonnnj
Copy link
Member Author

mcdonnnj commented Jun 7, 2021

Good improvement, though I'd recommend adding a comment explaining why setup.py is not **/setup.py.

Excellent suggestion. I've added a comment to every cache key declaration in 449eef4. Please let me know if you have any feedback about the comment.

@dav3r
Copy link
Member

dav3r commented Jun 7, 2021

Good improvement, though I'd recommend adding a comment explaining why setup.py is not **/setup.py.

Excellent suggestion. I've added a comment to every cache key declaration in 449eef4. Please let me know if you have any feedback about the comment.

Looks perfect to me - thanks for adding that.

@mcdonnnj mcdonnnj added the blocked This issue or pull request is awaiting the outcome of another issue or pull request label Jun 10, 2021
@mcdonnnj mcdonnnj merged commit 30b1756 into develop Jul 22, 2021
@mcdonnnj mcdonnnj deleted the improvement/add_setup_to_gha_cache_key branch July 22, 2021 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue or pull request is awaiting the outcome of another issue or pull request improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
Development

Successfully merging this pull request may close these issues.

3 participants