-
-
Notifications
You must be signed in to change notification settings - Fork 661
New external tool upgrade script #22064
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
base: main
Are you sure you want to change the base?
Conversation
This reverts commit f87cb21.
0d1af21
to
a8de875
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for making this! I think we might need to have a mention/deprecation period for the change of the format of the version.
@@ -42,8 +41,8 @@ class PexCli(TemplatedExternalTool): | |||
name = "pex" | |||
help = "The PEX (Python EXecutable) tool (https://github.com/pex-tool/pex)." | |||
|
|||
default_version = "v2.33.1" | |||
default_url_template = "https://github.com/pex-tool/pex/releases/download/{version}/pex" | |||
default_version = "2.33.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tfsec, we added a deprecation notice for changing the format of the version specifier, and then we used a function to convert it during the transition. We should at least call it out in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, created a separate pr #22090
parser.add_argument( | ||
"-w", | ||
"--workers", | ||
default=32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also use multiprocessing.cpu_count()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lilatomic Thank you for review! Ready for another round
@@ -42,8 +41,8 @@ class PexCli(TemplatedExternalTool): | |||
name = "pex" | |||
help = "The PEX (Python EXecutable) tool (https://github.com/pex-tool/pex)." | |||
|
|||
default_version = "v2.33.1" | |||
default_url_template = "https://github.com/pex-tool/pex/releases/download/{version}/pex" | |||
default_version = "2.33.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, created a separate pr #22090
parser.add_argument( | ||
"-w", | ||
"--workers", | ||
default=32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
The pr was generated with this command: ``` pants run build-support/bin:external-tool-upgrade -- -v src/python/pants/backend/ ``` (Notes are not generated yet) The tool code is here #22064 --------- Co-authored-by: Huon Wilson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry I dropped the ball on this! It looks good, I just missed that we also changed the version format for the Cue backend. Can you also split that out to a #22090 ?
@@ -25,14 +25,14 @@ class Cue(TemplatedExternalTool): | |||
Homepage: https://cuelang.org/ | |||
""" | |||
) | |||
default_version = "v0.4.3" | |||
default_version = "0.4.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed this the first time. Same as #22064 (comment)
It's hard to upgrade external tools versions, because it's not supported by tools like dependabot. This new script solves this problem, it fetches the latest release from github.com or k8s.io and modifies
default_known_versions
in python sources code.Example of upgraded tools here #22065