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

Unpin pip version and bump actions versions #3598

Merged
merged 10 commits into from
Feb 6, 2024
Merged

Unpin pip version and bump actions versions #3598

merged 10 commits into from
Feb 6, 2024

Conversation

ankatiyar
Copy link
Contributor

@ankatiyar ankatiyar commented Feb 5, 2024

Description

Partially fix kedro-org/kedro-plugins#674
(Also needs to be updated on kedro-plugins in a separate PR)

pip released a new version -> https://pypi.org/project/pip/24.0/

Development notes

  • Update versions of checkout, setup-python, and setup-msbuild actions
    Screenshot 2024-02-05 at 17 23 53

  • unpin pip version

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

  • Read the contributing guidelines
  • Signed off each commit with a Developer Certificate of Origin (DCO)
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes
  • Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@astrojuanlu
Copy link
Member

🤞🏽

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar ankatiyar changed the title Unpin pip version Unpin pip version and bump actions versions Feb 5, 2024
@ankatiyar ankatiyar self-assigned this Feb 5, 2024
@ankatiyar ankatiyar marked this pull request as ready for review February 5, 2024 17:32
@ankatiyar ankatiyar requested a review from merelcht as a code owner February 5, 2024 17:32
@ankatiyar ankatiyar requested a review from noklam February 5, 2024 17:32
ankatiyar and others added 3 commits February 5, 2024 17:36
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
…npin-pip

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Copy link
Contributor

@noklam noklam left a comment

Choose a reason for hiding this comment

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

I've gone through the Release note of pip

Normalize extras according to PEP 685 from package metadata in the resolver for comparison. This ensures extras are correctly compared and merged as long as the package providing the extra(s) is built with values normalized according to the standard. Note, however, that this does not solve cases where the package itself contains unnormalized extra values in the metadata. (#11649)

Noted the original problem was related to the unnormalised extra names we have in kedro-datasets. I suspect the tests passed here because we don't have dataset extras in kedro anymore. So I suggest when you try to make the same changes in kedro-datasets, make sure things like kedro-datasets[all] will work as expected, right now I think it will most likely fail.

https://peps.python.org/pep-0685/

@@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the node 16 actions have been deprecated by Github Actions and the new versions of these actions have transitioned to node 20 (https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/)

@ankatiyar
Copy link
Contributor Author

I've gone through the Release note of pip

Normalize extras according to PEP 685 from package metadata in the resolver for comparison. This ensures extras are correctly compared and merged as long as the package providing the extra(s) is built with values normalized according to the standard. Note, however, that this does not solve cases where the package itself contains unnormalized extra values in the metadata. (#11649)

Noted the original problem was related to the unnormalised extra names we have in kedro-datasets. I suspect the tests passed here because we don't have dataset extras in kedro anymore. So I suggest when you try to make the same changes in kedro-datasets, make sure things like kedro-datasets[all] will work as expected, right now I think it will most likely fail.

https://peps.python.org/pep-0685/

I'm trying it out on kedro-datasets but i'll leave the ticket open if it doesn't work out.

@ankatiyar ankatiyar merged commit 9b11c59 into main Feb 6, 2024
28 checks passed
@ankatiyar ankatiyar deleted the unpin-pip branch February 6, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the temporary pin for pip for e2e tests and .readthedocs.yml
4 participants