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
Create python wheel #915
base: dev
Are you sure you want to change the base?
Create python wheel #915
Conversation
Autopkg currently delivers the autopkg python module as a console script. This makes it near impossible to package the project into a wheel. This PR intends to do the following: - maintain backwards compatibility be creating a console script as close to as setuptools would - move autopkg module into the autopkgcli - fix test that failed due to move see: https://stackoverflow.com/questions/9133526/git-recover-failed-commits-message https://peps.python.org/pep-0338/
[project] | ||
name = "autopkg" | ||
authors = [] | ||
version = "2.7.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be sourced from another file? I know it is technically possible, but would be nice if the version was single sourced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like AutoPkg does not have the current version of itself in the repo in a file, only in the git tags / releases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version is in plist which is incompatible with setuptools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is the plist that contains the version? I was looking for it.
autopkg = "autopkgcli.autopkg:main" | ||
|
||
[tool.setuptools.dynamic] | ||
dependencies = {file = ["gh_actions_requirements.txt"]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this, the other dependency files have too much stuff in them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot in the current requirements that could be qualified as dev tools and not actually needed for the running of app. This should be trimmed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree, it makes sense to make a distinction between requirements for run/build vs requirements for dev/test/validation. I do think there is too much stuff in some of the requirements files for the sake of doing the basics in a github action or just running recipes on a build server.
I think the reason for the additional items in the requirements is because the requirements were traditionally only ever used by people doing dev work, while the production way to use autopkg was to install the PKG file on MacOS.
The biggest change this incurs is moving the main code from |
import plistlib | ||
import sys | ||
import unittest | ||
from textwrap import dedent | ||
from unittest.mock import mock_open, patch | ||
|
||
import autopkgcli.autopkg # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will have an issue if you are trying to dev/test autopkg source, then this will actually load autopkg from the pip installed version, not from the source version you are currently testing as a cloned repo.
I think this should only be a problem if you have previously pip installed autopkg? or maybe it only works if you have pip installed autopkg and won't work otherwise? I don't remember exactly.
autopkg = imp.load_source( | ||
"autopkg", os.path.join(os.path.dirname(__file__), "..", "autopkg") | ||
) | ||
from autopkgcli import autopkg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue: I think this will have an issue if you are trying to dev/test autopkg source, then this will actually load autopkg from the pip installed version, not from the source version you are currently testing as a cloned repo.
I think this should only be a problem if you have previously pip installed autopkg? or maybe it only works if you have pip installed autopkg and won't work otherwise? I don't remember exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not if you start using env python as your dev python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this get merged, additional PRs will obviously need to be done to update docs on install and dev environment setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well I don't think using this method is required especially for the default use case on MacOS in which case the PKG would still be preferred.
@nmcspadden I think you have to approve this PR to run the github action automated tests. |
The unit test failed due to: github.GithubException.RateLimitExceededException: 403 {"message": "API rate limit exceeded for 13.105.49.65. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)" it seems like the github token isn't being read from the default location if no default location is specified, which then causes this issue. This is due to a different bug. See here: #899 |
I do think the fact that this change moves the core autopkg code from one file to another is a pretty significant change, and one that might not make sense at the moment with the potential impact it could have. I think this would be an impediment to getting autopkg 3.x out the door as the main release version, which I think is higher priority than the potential problems this change could introduce at the moment. I do like this concept in general though and do think it is worth future consideration. |
Make it possible to produce a python wheel form this project.
Benefits:
Known deficiencies: