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 phylum #22

Merged
merged 27 commits into from
Apr 25, 2022
Merged

Install phylum #22

merged 27 commits into from
Apr 25, 2022

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented Apr 22, 2022

Overview

This PR adds the phylum-init script entry point to the phylum package. The package was renamed from phylum-ci to phylum as a result of a successful PEP 541 Request: phylum to acquire the project name in PyPI. The initial goal, as specified in #5, was to replicate the behavior of the existing install-phylum-latest-action GitHub Action within the Python package. This was achieved and then exceeded a bit. It goes further in that phylum-init can be used to install a specified "target triple" and providing a Phylum token is not required. That makes the script usable for both the integrations and just on its own.

Performing the Minisign signature verification was not as easy as using an existing python implementation...mostly b/c one does not readily exist as a package on PyPI. A custom implementation was written and annotated to make it abundantly clear that it's not the best option, has a number of assumptions, and should not be used as a library for any other projects.

Here are some potential features to add to the CLI (pulled from comments in the code):

  • Add logging support, a verbosity option to control it, and swap out print statements for logging
  • Check for valid versions by using the GitHub API to compare against actual releases
    • If so, also programmatically generate the SUPPORTED_TARGET_TRIPLES
  • Use the GitHub API ("https://api.github.com") to more programmatically get the archive URL
  • Add a --list option, to show which versions are available
  • Add an option to account for pre-releases

EDIT: These have been turned into issues and removed from the code as comments:

Test coverage is just getting off the ground and, while the structure was added during this PR, the numbers are not where they should be:

================================ test session starts ================================
platform darwin -- Python 3.7.13, pytest-7.1.1, pluggy-1.0.0
cachedir: .tox/py37/.pytest_cache
rootdir: /Users/coggins/dev/phylum/phylum-ci
plugins: github-actions-annotate-failures-0.1.6, cov-3.0.0
collected 12 items

tests/functional/test_init.py ..                                             [ 25%]
tests/unit/test_package_metadata.py .....                                    [ 66%]
tests/unit/test_sig.py ....                                                  [100%]

---------- coverage: platform darwin, python 3.7.13-final-0 ----------
Name                                                            Stmts   Miss  Cover
-----------------------------------------------------------------------------------
.tox/py37/lib/python3.7/site-packages/phylum/__init__.py           10      0   100%
.tox/py37/lib/python3.7/site-packages/phylum/__main__.py            2      0   100%
.tox/py37/lib/python3.7/site-packages/phylum/init/__init__.py       1      0   100%
.tox/py37/lib/python3.7/site-packages/phylum/init/__main__.py       2      0   100%
.tox/py37/lib/python3.7/site-packages/phylum/init/cli.py          136     94    31%
.tox/py37/lib/python3.7/site-packages/phylum/init/sig.py           46     37    20%
-----------------------------------------------------------------------------------
TOTAL                                                             197    131    34%


================================== 12 passed in 0.68s ================================
__________________________________________ summary ___________________________________
  py37: commands succeeded
  congratulations :)

Another issue will be used (#11) or created to increase that coverage.

Checklist

  • Does this PR have an associated issue?
  • Have you ensured that you have met the expected acceptance criteria?
  • Have you created sufficient tests?
  • Have you updated all affected documentation?
  • Have you updated the CHANGELOG?
  • Have you applied labels to this PR, to usually include at least one Semver-* label?

Issue

Closes #5

maxrake added 22 commits April 18, 2022 14:03
With this change it is possible to call the package entrypoint as: `python -m phylum_ci`
There is at least a little bit of functionality provided by this package now and v0.1.0 is already taken/used
The PyPI request to take over the `phylum` package name was granted. This change makes use of that name, changing `phylum-ci` and `phylum_ci` to `phylum` everywhere that it makes sense. There are still some instances that should remain `phylum-ci` since that is what the GitHub repository is named.
The plan is to have multiple script entry points. This change accounts for the package structure needed to have them and populates the first one, phylum-install. It also refactors the test package into unit and functional tests.
* Add deps: cryptography, packaging, and pyyaml\n* Rename phylum.install package to phylum.init\n* Add initial functionality to fetch and install the Phylum CLI
The pyyaml dependency fails to install in Python 3.7 environments for Apple Silicon systems. This appears to be due to the default nature in which the sdist is attempted to be installed by Poetry and with the LibYAML c bindings. There is no apparent method to direct Poetry to use the `--without-libyaml` option when building from source. One advantage of switching to ruamel.yaml is that round-tripping is supported, which means order and comments are preserved with loading-modifying-dumping a YAML file.
…he token option

Making this change means the script won't fail if the `--token` option is not specified. This means the script can be used to install or re-install on systems that already have a registered user and existing settings.
@maxrake maxrake added enhancement New feature or request Semver-Minor labels Apr 22, 2022
@maxrake maxrake requested a review from a team as a code owner April 22, 2022 20:12
@maxrake maxrake requested a review from cd-work April 22, 2022 20:12
@maxrake maxrake self-assigned this Apr 22, 2022
kylewillmon
kylewillmon previously approved these changes Apr 22, 2022
README.md Outdated Show resolved Hide resolved
src/phylum/__main__.py Outdated Show resolved Hide resolved
src/phylum/init/cli.py Outdated Show resolved Hide resolved
src/phylum/init/cli.py Outdated Show resolved Hide resolved
src/phylum/init/cli.py Outdated Show resolved Hide resolved
src/phylum/init/cli.py Outdated Show resolved Hide resolved
@maxrake maxrake requested a review from kylewillmon April 23, 2022 02:19
src/phylum/init/cli.py Show resolved Hide resolved
src/phylum/init/cli.py Show resolved Hide resolved
src/phylum/init/cli.py Outdated Show resolved Hide resolved
src/phylum/init/cli.py Outdated Show resolved Hide resolved
@maxrake maxrake requested a review from kylewillmon April 25, 2022 16:42
@maxrake maxrake merged commit e74afba into develop Apr 25, 2022
@maxrake maxrake deleted the install_phylum branch April 25, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replicate existing install-phylum-latest-action GHA
2 participants