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

Update python dependencies #12152

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
4 participants
@cblecker
Copy link
Member

cblecker commented Apr 10, 2019

No description provided.

@cblecker

This comment has been minimized.

Copy link
Member Author

cblecker commented Apr 10, 2019

@stevekuznetsov @Katharine Oh, I saw that you both took care of this, this morning. Do we want to bump further, or just leave it where you have?

@fejta

fejta approved these changes Apr 10, 2019

Copy link
Contributor

fejta left a comment

/hold

@@ -1,3 +1,4 @@
Jinja2==2.10.1

This comment has been minimized.

Copy link
@fejta

fejta Apr 10, 2019

Contributor

Why does this differ from hack? Are you sure we need a specific version here?

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Apr 10, 2019

LGTM label has been added.

Git tree hash: 753d73a77933e37e8700d542f9b42264471d0851

@k8s-ci-robot k8s-ci-robot added the lgtm label Apr 10, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Apr 10, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, fejta

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cblecker cblecker force-pushed the cblecker:jinja branch from 75e50f8 to ec7e26a Apr 10, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Apr 10, 2019

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm label Apr 10, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

cblecker commented Apr 10, 2019

@fejta I.. couldn't find anything referencing or using hack/test_requirements.txt. gubernator has it's own, different test_requirements, and nothing in hack touches jinja.

so I just nuked the whole file.

@Katharine

This comment has been minimized.

Copy link
Member

Katharine commented Apr 10, 2019

I am somewhat disinclined to do this now, primarily because I spent my entire morning getting gubernator to behave again after bumping the one dependency to a fixed version and have no particular desire to do it again today.

To my knowledge, we have no pressing reason to do this.

@cblecker

This comment has been minimized.

Copy link
Member Author

cblecker commented Apr 10, 2019

@Katharine I'm okay with it.. the security issue is resolved by what's already in the repo. WDYT about just dropping the hack/test_requirements.txt file? Do you know of any use for it?

@fejta

This comment has been minimized.

Copy link
Contributor

fejta commented Apr 10, 2019

Do you know of any use for it?

Yes we need it to pass unit tests (I don't think we need a different version of things though)

@cblecker

This comment has been minimized.

Copy link
Member Author

cblecker commented Apr 10, 2019

works for me. I'll just close this :)

@cblecker cblecker closed this Apr 10, 2019

@cblecker cblecker deleted the cblecker:jinja branch Apr 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.