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

Install cisagov/skeleton-python-library directly with pip #188

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Feb 20, 2024

🗣 Description

This pull request updates the installation of cisagov/skeleton-python-library to be installed directly with pip.

Note

I am creating this pull request against a branch that will represent a larger body of work. This branch will later have a pull request against develop once the body of work is completed.

Warning

This change breaks the Docker composition's ability to change the secret message that is printed. I am hoping to restore this functionality by changing part of how cisagov/skeleton-python-library works in the future.

💭 Motivation and context

There is no reason we shouldn't let pip handle the retrieval, decompression, and installation of a source package on its own. This is a cleaner approach and involves fewer dependencies in the generated Docker image which in turn gives us a smaller image (130MiB vs. 223MiB).

🧪 Testing

Automated tests pass.

✅ Pre-approval 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.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

Instead of downloading the source archive, extracting it, and then
installing it with pip we instead just let pip directly install the
package.
Since we are now installing cisagov/skeleton-python-library directly
with pip we no longer need these OS packages.
Now that we are not overwriting the internal Python package file the
text we look for must match what is output by default. The Docker
Compose secret configuration is left in place to continue to serve as
an example and to be leveraged for a future update to
cisagov/skeleton-python-library that can provide similar functionality
to what was removed in this project.
@mcdonnnj mcdonnnj added breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use labels Feb 20, 2024
@mcdonnnj mcdonnnj self-assigned this Feb 20, 2024
@dav3r
Copy link
Member

dav3r commented Feb 21, 2024

@mcdonnnj Did you mean to enable auto-merge on this skeleton PR?

@dav3r
Copy link
Member

dav3r commented Feb 21, 2024

@mcdonnnj Did you mean to enable auto-merge on this skeleton PR?

Ah, my bad - I see that this PR is not to the develop branch.

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.

👍 👍

@mcdonnnj mcdonnnj merged commit aa39972 into improvement/update_Dockerfile_configuration Feb 21, 2024
16 checks passed
@mcdonnnj mcdonnnj deleted the improvement/install_skeleton-python-library_directly branch February 21, 2024 13:50
@mcdonnnj
Copy link
Member Author

@mcdonnnj Did you mean to enable auto-merge on this skeleton PR?

Ah, my bad - I see that this PR is not to the develop branch.

Ah, sorry I forgot to include the blurb about branches that I put in #187 🤦

@mcdonnnj mcdonnnj mentioned this pull request Mar 6, 2024
9 tasks
cisagovbot pushed a commit that referenced this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This issue or pull request involves changes to existing functionality 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