Integrate skeleton-python-library and Migrate to Python 3 #2

mcdonnnj · 2020-11-11T04:13:48Z

🗣 Description

This PR merges histories with cisagov/skeleton-python-library to bring this project into the modern era. This also results in dropping Python 2 support and becoming a Python 3 project. This closes #1 .

This PR should only be merged along with the following PRs:
cisagov/cyhy_amis#292
https://github.com/jsf9k/cyhy-commander/pull/17

Opinions Wanted

I would like to remove the Dockerfile from this repo as it is not used as far as I am aware. If a Docker configuration was required I believe it would be straightforward to create a new project based on cisagov/skeleton-docker to fill that need.

💭 Motivation and Context

Python 2 support is very dead, so any Python projects we are actively using need to migrate to Python 3. This project is not dependent on other CyHy projects, so it can migrate to Python 3 without issue.

This will also allow instances that use only this project (like the portscan and vulnscan instances in the CyHy environment) to drop Python 2 altogether and use a recent OS version (such as Debian Buster).

🧪 Testing

I deployed portscan and vulnscan instances in a cisagov/cyhy_amis testing environment and was able to send work and retrieve results successfully.

✅ Checklist

This PR has an informative and human-readable title.
All relevant type-of-change labels have been added.
I have read the CONTRIBUTING document.
These code changes follow cisagov code standards.
All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
All new and existing tests pass.

Switch CI from Travis to GitHub actions

…t_autoupdate Add updates from running pre-commit autoupdate

See: ansible/ansible-lint#590

Replace Travis-CI with GitHub actions

Split workflow into multiple jobs. Add artifact output.

Re-enable ansible-lint and autoupgrade.

…nsible-lint and the updated version is included in the version being used now.

Pull upstream changes from skeleton-generic.

These seem to be very large caches. Restoring an old one and updating it results in a cache larger than the maximum allowed cache size: 200MB. "Cache size of 254757924 bytes is over the 200MB limit, not saving cache." So if the config changes it is best to just take the cache-miss and start from scratch.

Add GitHub action caching of pre-commit hooks and pip packages.

Add caches for GitHub Actions.

jsf9k

Approved, but please see my comments. In particular, please see my comment regarding preserving repository history.

jsf9k · 2020-11-13T14:55:48Z

Dockerfile

-FROM python:2
-MAINTAINER Mark Feldhousen <mark.feldhousen@hq.dhs.gov>
+FROM python:3
+MAINTAINER Mark Feldhousen <mark.feldhousen@cisa.dhs.gov>


I know we are thinking of getting rid of this file, but please note that this is no longer the preferred way to specify the maintainer. You should use a label instead. @felddy recently had a PR (I can't find that PR right now - can you provide a link @felddy?) where he used the set of labels specified by the Open Container Initiative](https://opencontainers.org/). See here, for example. The OCI label standard supersedes the other standard in common use, which was Label Schema.

I believe the PR you are looking for is cisagov/skeleton-docker#38, specifically this Dockerfile bit.

I believe the PR you are looking for is cisagov/skeleton-docker#38, specifically this Dockerfile bit.

Yep, that's the one!

jsf9k · 2020-11-13T14:59:14Z

setup.py

+    download_url="https://github.com/cisagov/cyhy-runner",
+    # Author details
+    author="Cyber and Infrastructure Security Agency",
+    author_email="ncats@hq.dhs.gov",


Updated the issue to mention updating the URL as well.

jsf9k · 2020-11-13T14:59:59Z

src/cyhy_runner/__init__.py

+# Standard Python Libraries
+from typing import List
+
+from ._version import __version__  # noqa: F401


A comment here explaining what F401 is would be good. I recognize that this change might be better made upstream.

I've added a suggestion for this for ease of resolution

Yes, this should be updated in cisagov/skeleton-python-library as these two lines are related and both lack explanation:
https://github.com/cisagov/skeleton-python-library/blob/0f6efe072d3045b2dd97a190ef498f4ba89303b7/src/example/__init__.py#L2
https://github.com/cisagov/skeleton-python-library/blob/0f6efe072d3045b2dd97a190ef498f4ba89303b7/setup.py#L29

I created an issue upstream to flesh them both out: cisagov/skeleton-python-library#58

src/cyhy_runner/cyhy_runner.py

hillaryj · 2020-11-13T17:10:33Z

src/cyhy_runner/__init__.py

+# Standard Python Libraries
+from typing import List
+
+from ._version import __version__  # noqa: F401


Suggested change

from ._version import __version__ # noqa: F401

# Module imported but unused - ignored to preserve skeleton functionality

from ._version import __version__ # noqa: F401

It is not directly used, which is what makes flake8 mad, but it populates cyhy_runner.__version__, which is used to get version information about the package. A comment fleshing out why it's got a noqa on it should be added to cisagov/skeleton-python-library. This is related to how we've chosen to SSoT the package version from the options recommended by Python packaging conventions.

src/cyhy_runner/_version.py

hillaryj

🦖

Sort the install_requires and extras_require["test"] arguments by key.

lgtm-com · 2020-11-13T19:08:41Z

This pull request fixes 3 alerts when merging 01fcc26 into 74d2cfb - view on LGTM.com

fixed alerts:

3 for Unused import

Group related pre-commit hooks together. Make sure that hooks are alphabetically sorted within those groups.

Switch to the file line reading version of extracting the version from https://packaging.python.org/guides/single-sourcing-package-version/ instead of the exec method on the same page. The exec method required us to use a "# nosec" to manually disable Bandit checking on that line. Although that method is more straightforward, I do not feel that it is worth using an exec in the codebase when another option is available.

Organize and Sort pre-commit Hooks

…gihub-dir Ensure that the cisagov devs are the owners of the .github directory

…re-simply Use the python version output by actions/setup-python

Also updated the test and build jobs in the build workflow to mirror cache key format changes in the lint job.

⚠️ CONFLICT! Lineage pull request for: skeleton

Sort the Package Requirements in setup.py

Remove or Explain Manual Testing Check Disables

…into skeletonize

lgtm-com · 2020-12-02T15:15:08Z

This pull request fixes 3 alerts when merging 514e491 into 74d2cfb - view on LGTM.com

fixed alerts:

3 for Unused import

I believe that this was missed when #3 was merged due to timing between when this change was made in cisagov/skeleton-python-library#51 and when #2 was merged.

felddy and others added 30 commits October 17, 2019 16:21

Correct capitalization for badge.

ff27a9a

Make action badge a link.

58c707d

Lowercasing workflow name so the shield matches all others.

f8ca5c6

Remove travis. :(

da7e1cc

Add comment about why we are pinning virtualenv

e666a6f

Rename env variable to make sense in a post-travis world.

e4127dd

Formatting

038a75a

Merge pull request #20 from cisagov/improvement/actions

fae7694

Switch CI from Travis to GitHub actions

Add updates from running pre-commit autoupdate

78a230d

Merge pull request #18 from cisagov/improvement/updates_from_precommi…

b189690

…t_autoupdate Add updates from running pre-commit autoupdate

Replace Travis-CI with GitHub actions.

05a136b

Fix action shield link

02fb008

Disable ansible-lint until they fix their setup.py

b31ad85

See: ansible/ansible-lint#590

Remove extra line.

b11e39a

Merge pull request #20 from cisagov/improvement/actions

12b91c4

Replace Travis-CI with GitHub actions

Split workflow into multiple jobs. Add artifact output.

df0d922

Merge pull request #22 from cisagov/improvement/split_actions

7ab251e

Split workflow into multiple jobs. Add artifact output.

Re-enabled ansible-lint and ran pre-commit autoupgrade.

3c78dd9

Merge pull request #21 from cisagov/pre-commit_autoupdate_2019-11-02

7dfd8bf

Re-enable ansible-lint and autoupgrade.

Pull upstream changes from skeleton-generic.

af4e9fe

Remove virtualenv version pinning as the issue has been resolved in a…

36f1b3b

…nsible-lint and the updated version is included in the version being used now.

Merge pull request #23 from cisagov/pull_upstream_changes_2019-11-05

414b6fc

Pull upstream changes from skeleton-generic.

Add GitHub action caching of pre-commit hooks and pip packages.

5d5567d

Merge branch 'develop' into improvement/action_cache

c26ea01

Merge pull request #22 from cisagov/improvement/action_cache

10bbfdf

Add GitHub action caching of pre-commit hooks and pip packages.

Add caches for GitHub Actions.

cf99067

Pass in the github release tag for testing.

b10dd19

Merge pull request #25 from cisagov/improvement/action_cache

ff684c0

Add caches for GitHub Actions.

Merge branch 'develop' into bug/release_tag

5f6a413

jsf9k approved these changes Nov 13, 2020

View reviewed changes

hillaryj reviewed Nov 13, 2020

View reviewed changes

src/cyhy_runner/_version.py Show resolved Hide resolved

hillaryj approved these changes Nov 13, 2020

View reviewed changes

mcdonnnj added 2 commits November 13, 2020 13:37

Make sure the requirements in setup.py are sorted

a9ee3dd

Sort the install_requires and extras_require["test"] arguments by key.

Remove the unused Dockerfile configuration

01fcc26

mcdonnnj and others added 15 commits November 15, 2020 23:02

Organize pre-commit hooks

64c3fb6

Group related pre-commit hooks together. Make sure that hooks are alphabetically sorted within those groups.

Add comment explaining a Flake8 check disable

40b3b80

Resolve LGTM alert

57fc8a4

Merge pull request #60 from cisagov/organize_pre-commit_requirements

763d247

Organize and Sort pre-commit Hooks

Merge branch 'develop' into improvement/insist-that-devs-own-gihub-dir

1310cfd

Merge pull request #57 from cisagov/improvement/insist-that-devs-own-…

2a2067f

…gihub-dir Ensure that the cisagov devs are the owners of the .github directory

Merge branch 'develop' into improvement/get-python-version-more-simply

b638143

Merge pull request #59 from cisagov/improvement/get-python-version-mo…

3b312c4

…re-simply Use the python version output by actions/setup-python

Merge github.com:cisagov/skeleton-generic into lineage/skeleton

dd6e0b8

Also updated the test and build jobs in the build workflow to mirror cache key format changes in the lint job.

Merge pull request #61 from cisagov/lineage/skeleton

a2e4fd3

⚠️ CONFLICT! Lineage pull request for: skeleton

Merge branch 'develop' into sort_requirements

a8f761c

Merge pull request #59 from cisagov/sort_requirements

a8bdbe5

Sort the Package Requirements in setup.py

Merge branch 'develop' into resolve_manual_check_disables

0747d20

Merge pull request #60 from cisagov/resolve_manual_check_disables

b32bb9a

Remove or Explain Manual Testing Check Disables

mcdonnnj mentioned this pull request Nov 27, 2020

2020-11 Redeploy CyHy Environment cisagov/cyhy_amis#293

Closed

11 tasks

Merge branch 'develop' of github.com:cisagov/skeleton-python-library …

514e491

…into skeletonize

mcdonnnj merged commit 3f21365 into develop Dec 2, 2020

mcdonnnj deleted the skeletonize branch December 2, 2020 18:18

mcdonnnj added a commit that referenced this pull request Aug 4, 2021

Apply previous skeleton changes that were missed

5323052

I believe that this was missed when #3 was merged due to timing between when this change was made in cisagov/skeleton-python-library#51 and when #2 was merged.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate skeleton-python-library and Migrate to Python 3 #2

Integrate skeleton-python-library and Migrate to Python 3 #2

mcdonnnj commented Nov 11, 2020 •

edited

Loading

jsf9k left a comment

jsf9k Nov 13, 2020

dav3r Nov 13, 2020

jsf9k Nov 13, 2020

jsf9k Nov 13, 2020

jsf9k Nov 13, 2020

hillaryj Nov 13, 2020

mcdonnnj Nov 13, 2020

mcdonnnj Nov 13, 2020

hillaryj Nov 13, 2020

mcdonnnj Nov 13, 2020

hillaryj left a comment

lgtm-com bot commented Nov 13, 2020

lgtm-com bot commented Dec 2, 2020

	from ._version import __version__ # noqa: F401
	# Module imported but unused - ignored to preserve skeleton functionality
	from ._version import __version__ # noqa: F401

Integrate skeleton-python-library and Migrate to Python 3 #2

Integrate skeleton-python-library and Migrate to Python 3 #2

Conversation

mcdonnnj commented Nov 11, 2020 • edited Loading

🗣 Description

Opinions Wanted

💭 Motivation and Context

🧪 Testing

✅ Checklist

jsf9k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hillaryj left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Nov 13, 2020

lgtm-com bot commented Dec 2, 2020

mcdonnnj commented Nov 11, 2020 •

edited

Loading