-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Update release artifact download urls #26470
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
Conversation
| 'jdbc': ('trino-jdbc', 'jar', None), | ||
| 'server': ('trino-server', 'tar.gz'), | ||
| 'server-core': ('trino-server-core', 'tar.gz'), | ||
| 'cli': ('trino-cli', None), |
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.
Is this going to work? The filename should be like https://repo1.maven.org/maven2/io/trino/trino-cli/476/trino-cli-476-executable.jar
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.
No, it can be anything we want. We’ll publish those artifacts to github, so we’ll change the name. No need for the “-executable” qualifier
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.
Ok .. so the filename will be just trino-cli-476.jar ? If so I have to adjust my PR for the website .. which is fine by my .. just might be confusing for readers of the book or older docs for what they are used to. Just let me know so I can adjust the other PR
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.
No, it will be just “trino-cli-476”. It’s just a binary executable
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.
Uff .. I think thats actually a bad idea. It is not a pure executable .. it still requires a JVM on the path .. and with the .jar extension it can be recognized and run .. without the extension that is harder to figure out and might not work so well.
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.
Kind of feels like a unrelated change that we should not pull in unless we have to.
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.
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.
The extension doesn’t actually do anything. The file behaves like a script with a shebang line.
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.
It helps if you are running on Windows (I'm not sure whether we support it or not). We could add a check in the cli itself, to check for required flags if it's run via java -jar).
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 guess either way should be fine. The instructions in the docs already talk about renaming it so it doesnt matter I guess - see https://trino.io/docs/current/client/cli.html
We just have to adjust the example for Windows in that same doc since it uses the old -executable.jar
|
Please confirm about expected filenames and then also review and comment in trinodb/trino.io#811 since that assumes identical filenames to what we had in Central |
mosabua
left a comment
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 am good with either approach for the CLI filename .. the rest if fine anyway.. so I am approving.
Going forward, binary artifacts will be available under GitHub releases
77c0131 to
3fbcd17
Compare
Going forward, binary artifacts will be available under GitHub releases
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.