-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Transition CI/CD to GitHub Workflows #2378
Conversation
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
setup.py
Outdated
classifiers=[ | ||
"Development Status :: 5 - Production/Stable", | ||
"Intended Audience :: Developers", | ||
"License :: OSI Approved :: Apache Software License", | ||
"Programming Language :: Python", | ||
"Programming Language :: Python :: 2", |
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.
why drop the python2 support notice?
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.
GitHub dropped support for testing Python 2.7 back in June as explained in actions/runner-images#7401. I figure it'd make sense to drop support for it if we can't test it on any runners in our CI/CD workflows.
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 we're dropping support, we should probably add python_requires
to the setup.py file.
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'm 👍 on dropping python 2 support (in fact I'm all for aligning with upstream Python EOL cadence, as that seems pretty conservative to have 5 years to support a version), but I'd do it separate from this PR... keep this one focused on the CI workflow migration.
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.
Sounds good, I'll revert my tag drop for now.
Thanks for working on this, really appreciated! I'll try to give a more thorough review tomorrow, but if I don't get to it, please merge w/o waiting for me... |
I should create a separate PR at some point for a workflow that will generate docs and push to readthedocs for each release. I've written similar workflows for GitHub Pages, I just need to know what our authentication situation is for readthedocs. |
coveralls==2.1.2 | ||
crc32c==2.1 | ||
docker-py==1.10.6 | ||
flake8==3.8.3 | ||
lz4==3.1.0 | ||
mock==4.0.2 | ||
py==1.9.0 | ||
pylint==2.6.0 | ||
pytest==6.0.2 | ||
pytest-cov==2.10.1 | ||
pytest-mock==3.3.1 | ||
pytest-pylint==0.17.0 | ||
python-snappy==0.5.4 | ||
Sphinx==3.2.1 | ||
sphinx-rtd-theme==0.5.0 | ||
tox==3.20.0 | ||
xxhash==2.0.0 | ||
coveralls | ||
crc32c | ||
docker-py | ||
flake8 | ||
lz4 | ||
mock | ||
py | ||
pylint | ||
pytest | ||
pytest-cov | ||
pytest-mock | ||
pytest-pylint | ||
python-snappy | ||
Sphinx | ||
sphinx-rtd-theme | ||
tox | ||
xxhash |
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.
In my personal opinion, we don't necessarily need to pin versions until we get all of our workflows in order. We can revisit version pinning at a later point, many of these pinned versions are outdated.
py.test {posargs:--pylint --pylint-rcfile=pylint.rc --pylint-error-types=EF --cov=kafka --cov-config=.covrc} | ||
pytest {posargs:--pylint --pylint-rcfile=pylint.rc --pylint-error-types=EF --cov=kafka --cov-config=.covrc} | ||
setenv = | ||
CRC32C_SW_MODE = auto | ||
PROJECT_ROOT = {toxinidir} | ||
passenv = KAFKA_VERSION | ||
|
||
[testenv:py26] | ||
# pylint doesn't support python2.6 | ||
commands = py.test {posargs:--cov=kafka --cov-config=.covrc} | ||
|
||
[testenv:pypy] | ||
# pylint is super slow on pypy... | ||
commands = py.test {posargs:--cov=kafka --cov-config=.covrc} | ||
commands = pytest {posargs:--cov=kafka --cov-config=.covrc} |
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.
Calls to py.test
are deprecated in favor of pytest
, https://stackoverflow.com/a/41893170/9852671 provides a useful explanation.
all_topics = set(['t{}'.format(i) for i in range(1, n_topics + 1)]) | ||
all_topics = sorted(['t{}'.format(i) for i in range(1, n_topics + 1)]) |
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.
sorted
is a required call for Python 3.11+, otherwise the test fails.
- "0.8.2.2" | ||
- "0.9.0.1" | ||
- "0.10.2.2" | ||
- "0.11.0.2" | ||
- "0.11.0.3" | ||
- "1.1.1" | ||
- "2.4.0" | ||
- "2.5.0" | ||
- "2.6.0" |
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'd be nice to keep all of these versions in a file somewhere for convenience, so we don't need to edit supported Kafka versions in multiple places.
# the `language` matrix defined below to confirm you have the correct set of | ||
# supported CodeQL languages. | ||
# | ||
name: CodeQL |
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 critical to include this, but it has served me well in other projects.
Just want to check with you both before I merge this. I'm keen on getting this into the main branch so I can test existing PRs against the pipeline and draft a workflow for docs generation. The only PRs I plan on merging are documentation improvements. I was thinking we do a release of the library in its current state now, and announce deprecation of older Python versions. What do you think? |
I'll go ahead and merge this since no core functionality is changed. |
* Create GH workflows to test code * Update tests for future Python versions
Placeholder text, will add more notes later.
This change is