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

chore: Bump pyyaml to version 6, revert pyyaml pins #5573

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

mildaniel
Copy link
Contributor

Which issue(s) does this change fix?

#5543

Why is this change necessary?

Fix an issue where there are multiple dependency conflicts causing issues with pip install.

How does it address the issue?

We've removed our dependency on serverlessrepo so that we can bump pyyaml.

What side effects does this change have?

N/A

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

@mildaniel mildaniel requested a review from a team as a code owner July 19, 2023 23:38
@mildaniel mildaniel enabled auto-merge July 19, 2023 23:58
@mildaniel mildaniel added this pull request to the merge queue Jul 20, 2023
Merged via the queue into aws:develop with commit 079a2f8 Jul 20, 2023
75 checks passed
@@ -7,7 +7,7 @@ Flask<2.3
boto3>=1.26.109,==1.*
jmespath~=1.0.1
ruamel_yaml~=0.17.32
PyYAML>=5.4.1,==5.*
PyYAML>=6.0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should be do ~=6.0,>=6.0.1?

This will get at least 6.0.1 but won't do major version bump expectedly

@mildaniel mildaniel deleted the bump-pyyaml branch July 20, 2023 15:44
@@ -695,17 +704,12 @@ sarif-om==1.0.4 \
--hash=sha256:539ef47a662329b1c8502388ad92457425e95dc0aaaf995fe46f4984c4771911 \
--hash=sha256:cd5f416b3083e00d402a92e449a7ff67af46f11241073eea0461802a3b5aef98
# via cfn-lint
serverlessrepo==0.1.10 \

Choose a reason for hiding this comment

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

Hello, thank you for your contribution.

Some days ago, I was thinking:

  • "why aws-sam-cli is utilizing serverlessrepo considering the current status (only-read | public archive)?"

I have some questions now:

  • How do you make sure that the package serverlessrepo is not been in use?;
  • Is there any other package with this same behaviour? (unused).

Seems to me that another non-used package soon or later "could" lead to another breaking. 👋

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @realFranco, the serverlessrepo package was in fact being used by the sam publish command. We brought it in to be vendored by us yesterday #5572. All other dependencies are being used.

Choose a reason for hiding this comment

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

Nice, thank you for the clarification.

bkahloon pushed a commit to bkahloon/amazon-emr-on-eks-custom-image-cli that referenced this pull request Sep 21, 2023
**py** vulnerability resolved via the fix mentioned in [this Dependabot fix](https://github.com/awslabs/amazon-emr-on-eks-custom-image-cli/security/dependabot/1), which is to update package pytest to `7.2.0` and remove dependency on `py` package. Which subsequently required updating PyYaml to `~=6.0,>=6.0.1` (based on [this aws-sam-cli fix](aws/aws-sam-cli#5573))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants