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

Add PyPI checker and issue creator #22807

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

anthonykim1
Copy link

Resolves: #22800

@anthonykim1 anthonykim1 added the debt Covers everything internal: CI, testing, refactoring of the codebase, etc. label Jan 30, 2024
@anthonykim1 anthonykim1 self-assigned this Jan 30, 2024
@karthiknadig
Copy link
Member

@anthonykim1 What we really need here is have a workflow that runs once a week. that workflow should install packages with --pre for pip. Then run our python test suite on it. If it fails, then create an issue.

If it creates an issue on every pre-release that will be a lot of noise. I think it should do this only if the python tests fail.

@anthonykim1
Copy link
Author

anthonykim1 commented Feb 1, 2024

Now mainly having two condition to trigger issue creation in Python Repo:

  1. There is newest version (including pre-releases) of Pytest that is different from what we have. This won't be too much of a noise since cron job runs once a week as recommended.
  2. Python tests fails with the --pre with pip installs.

@anthonykim1 anthonykim1 marked this pull request as ready for review February 27, 2024 19:03
@VSCodeTriageBot VSCodeTriageBot added this to the March 2024 milestone Feb 27, 2024
.github/workflows/notifier.yml Outdated Show resolved Hide resolved
import os
import pathlib
import importlib.metadata
import pytest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is pytest imported here? it won't even be installed when this script is actually called.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we would need this for importlib.metadata.version("pytest") to correctly fetch the version of pytest that is imported.
Should I add pytest in additional to python -m pip install -U pip wheel PyGithub or do we not need it at all??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your pipeline you are not installing pytest. import pytest will immediately fail. This works for you locally because you have pytest installed locally. Try running this from a fresh environment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need pytest then you need to install all requirements before running this script. In this script you can later update the installed pytest to whatever version you want.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see... the intent of importing pytest and running importlib.metadata.version("pytest") was to get the version of pytest that our python extension was using...

Now I'm little lost on how to correctly get this value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I install test-requirement.txt and then use importlib to dynaimically get it since test-requirement.txt does not pin to specific version of pytest.

.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
.github/workflows/pypi_notifier.py Show resolved Hide resolved
.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
.github/workflows/pypi_notifier.py Outdated Show resolved Hide resolved
@anthonykim1 anthonykim1 marked this pull request as draft February 29, 2024 01:15
@karthiknadig karthiknadig modified the milestones: March 2024, April 2024 Mar 28, 2024
@karthiknadig karthiknadig modified the milestones: April 2024, May 2024 Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Covers everything internal: CI, testing, refactoring of the codebase, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create GitHub Action to check PyPI and open issue
3 participants