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

fix(lambda-python-alpha): pipenv lock -r is no longer supported #28317

Merged
merged 3 commits into from
Dec 18, 2023

Conversation

stefanfreitag
Copy link
Contributor

As part of the pipenv release 2022.8.13 the deprecated way of generating requirements ("pipenv install -r" or "pipenv lock -r") has been removed in favor of the "pipenv requirements" command.

See #28015 for the motivation behind this change.

Closes #28015 .


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

As part of the pipenv release 2022.8.13 the deprecated way of generating requirements "pipenv install -r" or "pipenv lock -r" has been removed in favor of the "pipenv requirements" command.

* [Reference to pipenv CHANGELOG](https://github.com/pypa/pipenv/blob/main/CHANGELOG.md#2022813-2022-08-13)
* [Refernce to relevant pipenv pull request](pypa/pipenv#5200)

Fixes aws#28015 by implementing the proposed change.
@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK p2 labels Dec 10, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team December 10, 2023 09:24
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@stefanfreitag stefanfreitag changed the title fix(aws-lambda-python-alpha): pipenv lock -r is no longer supported fix(lambda-python-alpha): pipenv lock -r is no longer supported Dec 10, 2023
@stefanfreitag
Copy link
Contributor Author

Clarification Request

Hi, I would love to get feedback on two questions I have:

  • I am not sure if "fix" is the correct category for this changes.
    In sum, a deprecated and in newer pipenv versions removed configuration option is replaced by its successor. Would there be a more appropriate category?
  • The basic tests were updated, but I am unsure on the integration tests. According to the contribution guidelines I would need them. Would it be sufficient here to have the already existing integration tests executing successfully?

Thanks in advance,
Stefan

@aws-cdk-automation aws-cdk-automation added pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Dec 10, 2023
Copy link
Contributor

@lpizzinidev lpizzinidev left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
Looks good to me 👍

I am not sure if "fix" is the correct category for this changes.

I think that fix is the right tag for this PR.

The basic tests were updated, but I am unsure on the integration tests.

I think that the existing integration test should already provide coverage here.

@aws-cdk-automation aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Dec 12, 2023
@kaizencc
Copy link
Contributor

This breaks users of old versions of pipenv that don't have pipenv requirements. Looks like the version we have in our provided Dockerfile is pipenv==2022.4.8 because that is the latest that supports python3.6.

https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-lambda-python-alpha/lib/Dockerfile

CDK now officially only supports python3.8+ so we should update that to the latest version that supports 3.8. @stefanfreitag would you be willing to add that update to this PR? If not, I can circle back to it but I don't want to merge this PR before that update gets in.

@stefanfreitag
Copy link
Contributor Author

stefanfreitag commented Dec 15, 2023

Good morning @kaizencc,

I looked into pipenv 2022.4.8 but only in combination with python 3.11.6. Here "requirements" was already supported:

❯ pipenv --version
/home/stefan/temp/.venv/lib/python3.11/site-packages/pipenv/vendor/attr/_make.py:876: RuntimeWarning: Running interpreter doesn't sufficiently support code object introspection.  Some features like bare super() or accessing __class__ will not work with slotted classes.
  set_closure_cell(cell, cls)
pipenv, version 2022.4.8

❯ python --version
Python 3.11.6

❯ pipenv
/home/stefan/temp/.venv/lib/python3.11/site-packages/pipenv/vendor/attr/_make.py:876: RuntimeWarning: Running interpreter doesn't sufficiently support code object introspection.  Some features like bare super() or accessing __class__ will not work with slotted classes.
  set_closure_cell(cell, cls)
Courtesy Notice: Pipenv found itself running within a virtual environment, so it will automatically use that environment, instead of creating its own for any project. You can set PIPENV_IGNORE_VIRTUALENVS=1 to force pipenv to ignore that environment and create its own instead. You can set PIPENV_VERBOSITY=-1 to suppress this warning.
Usage: pipenv [OPTIONS] COMMAND [ARGS]...

Options:
  --where                         Output project home information.
  --venv                          Output virtualenv information.
  --py                            Output Python interpreter information.
  --envs                          Output Environment Variable options.
  --rm                            Remove the virtualenv.
  --bare                          Minimal output.
  --man                           Display manpage.
  --support                       Output diagnostic information for use in
                                  GitHub issues.
  --site-packages / --no-site-packages
                                  Enable site-packages for the virtualenv.
                                  [env var: PIPENV_SITE_PACKAGES]
  --python TEXT                   Specify which version of Python virtualenv
                                  should use.
  --three                         Use Python 3 when creating virtualenv.
  --clear                         Clears caches (pipenv, pip).  [env var:
                                  PIPENV_CLEAR]
  -q, --quiet                     Quiet mode.
  -v, --verbose                   Verbose mode.
  --pypi-mirror TEXT              Specify a PyPI mirror.
  --version                       Show the version and exit.
  -h, --help                      Show this message and exit.


Usage Examples:
   Create a new project using Python 3.7, specifically:
   $ pipenv --python 3.7

   Remove project virtualenv (inferred from current directory):
   $ pipenv --rm

   Install all dependencies for a project (including dev):
   $ pipenv install --dev

   Create a lockfile containing pre-releases:
   $ pipenv lock --pre

   Show a graph of your installed dependencies:
   $ pipenv graph

   Check your installed dependencies for security vulnerabilities:
   $ pipenv check

   Install a local setup.py into your virtual environment/Pipfile:
   $ pipenv install -e .

   Use a lower-level pip command:
   $ pipenv run pip freeze

Commands:
  check         Checks for PyUp Safety security vulnerabilities and against
                PEP 508 markers provided in Pipfile.
  clean         Uninstalls all packages not specified in Pipfile.lock.
  graph         Displays currently-installed dependency graph information.
  install       Installs provided packages and adds them to Pipfile, or (if no
                packages are given), installs all packages from Pipfile.
  lock          Generates Pipfile.lock.
  open          View a given module in your editor.
  requirements  Generate a requirements.txt from Pipfile.lock.
  run           Spawns a command installed into the virtualenv.
  scripts       Lists scripts in current environment config.
  shell         Spawns a shell within the virtualenv.
  sync          Installs all packages specified in Pipfile.lock.
  uninstall     Uninstalls a provided package and removes it from Pipfile.
  update        Runs lock, then sync.
  verify        Verify the hash in Pipfile.lock is up-to-date.

I was now going through the CHANGELOG of pipenv and noticed that requirements were introduced in v2022.4.8.
Here is the link to the release details: https://github.com/pypa/pipenv/releases/tag/v2022.4.8. From that perspective we should be fine then, or?
If so, we could decouple this change from the pipenv upgrade you mentionend - happy to pick this one up, too.

@kaizencc
Copy link
Contributor

Thanks for the research @stefanfreitag! I think we are good. If you are willing to take on the separate issue of upgrading the default version of pipenv to something sensible (3.8), please do!

Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

Thanks :)

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. and removed pr/needs-maintainer-review This PR needs a review from a Core Team Member pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. labels Dec 18, 2023
@stefanfreitag
Copy link
Contributor Author

@kaizencc Thanks for your feedback! Anything I need to do about the integrations tests for this PR (e.g. flagging as exemption request)?

@aws-cdk-automation aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Dec 18, 2023
@kaizencc kaizencc added pr-linter/exempt-integ-test The PR linter will not require integ test changes and removed pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run labels Dec 18, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review December 18, 2023 21:38

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 9ccf73c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

mergify bot commented Dec 18, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit f85f486 into aws:main Dec 18, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(aws-lambda-python-alpha): Pipenv lock -r is no longer supported
4 participants