Create a single binary distribution bundle #1589
Conversation
This script combines the Polaris admin tool and server distributions into a single package: Maintains separate admin and server components, provides a unified run script to launch either component, preserves all necessary dependencies and configurations, and simplifies deployment by having both components in one distribution.
… run script - Remove repository configuration from plugins, restore run.sh script, fix build issues
…rojects.main.properties - Remove run-script dependency from admin and server modules - Fix code formatting in build files
flyrain
force-pushed
the
merge-distributions
branch
from
3d708aa to
30e9e7a
Compare
| ├── README.md | ||
| ├── admin/ # Admin tool files | ||
| ├── server/ # Server files | ||
| └── run.sh |
There was a problem hiding this comment.
Spark was mentioned as an example, but IIRC, Spark's distribution has a bin directory for runnable files... Why not put run.sh into bin in Polaris too?
There was a problem hiding this comment.
I think one of reasons Spark has a dedicated bin directory is due to the number of items under the root directory like the following. Our distribution is different though, we don't have a lot of top level items, but I'm OK either way.
drwxr-xr-x@ 3 ygu staff 96B Aug 6 2024 yarn/
drwxr-xr-x@ 31 ygu staff 992B Aug 6 2024 sbin/
-rw-r--r--@ 1 ygu staff 166B Aug 6 2024 RELEASE
-rw-r--r--@ 1 ygu staff 4.5K Aug 6 2024 README.md
drwxr-xr-x@ 3 ygu staff 96B Aug 6 2024 R/
drwxr-xr-x@ 19 ygu staff 608B Aug 6 2024 python/
-rw-r--r--@ 1 ygu staff 56K Aug 6 2024 NOTICE
drwxr-xr-x@ 58 ygu staff 1.8K Aug 6 2024 licenses/
-rw-r--r--@ 1 ygu staff 22K Aug 6 2024 LICENSE
drwxr-xr-x@ 4 ygu staff 128B Aug 6 2024 kubernetes/
drwxr-xr-x@ 4 ygu staff 128B Aug 6 2024 examples/
drwxr-xr-x@ 6 ygu staff 192B Aug 6 2024 data/
drwxr-xr-x@ 8 ygu staff 256B Aug 6 2024 conf/
drwxr-xr-x@ 30 ygu staff 960B Aug 6 2024 bin/
drwxr-xr-x@ 255 ygu staff 8.0K Oct 14 2024 jars/
|
|
||
| script_dir="$(dirname "$0")" | ||
| # Default to server if no component specified | ||
| COMPONENT=${1:-server} |
There was a problem hiding this comment.
I'm not sure the default is useful. It is possible only if server is executed without arguments. 🤔
There was a problem hiding this comment.
Does server take any parameters? Even if yes, I think the no-arg run is still the majority, in that case, it's easier to use by just hit run.sh.
There was a problem hiding this comment.
why not have bin/polaris-server and bin/polaris-admin (note: no .sh suffix) similar to Spark's bin/spark-shell, bin/spark-sql, etc?..
There was a problem hiding this comment.
I think it's a good idea to have two different bash script, but I'd prefer to do that once we merge libs between admin tool and server(the third todo item)
There was a problem hiding this comment.
This PR refactors run scripts to support a combined dist archive anyway. I think it makes sense to put separate admin/server scripts in this PR.
|
|
||
| description = "Apache Polaris Binary Distribution" | ||
|
|
||
| val adminProject = project(":polaris-quarkus-admin") |
There was a problem hiding this comment.
As we do this change, I would suggest to rename polaris-quarkus-admin and polaris-quarkus-server as polaris-admin and polaris-server, and also maybe rename quarkus folder as dist folder or so.
There was a problem hiding this comment.
This change uses a bunch of highly discouraged Gradle practices:
- It's accessing another Gradle projects and those tasks - that's bad practice and highly discouraged.
- This leads to task-dependency and in turn build and Gradle caching issues
Please take a look at Gradle artifacts and configurations and defining portable artifact dependencies including the proper task dependencies implied by these mechanisms.
| @@ -0,0 +1,69 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
Nit: this file is placed under quarkus/distribution/distribution – is that intentional? I'd advocate for moving it one level up (it would become the README for the distribution module, but I think that's a good thing).
There was a problem hiding this comment.
That's intentional, which follows the same structure in admin and server module.
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/env bash | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
This is less portable than the original shebang.
There was a problem hiding this comment.
Good catch (I missed that)
|
@flyrain just to confirm, we are not planning to remove the individual distributions for server and tool, are we? |
Good question. We will remove the individual distribution once the single binary distribution is ready. Their jar file will still be published individually though. |
Are we sure about this? I assume that most people downloading the distribution would be interested in the server, not the tool. Imposing the combined distribution would mean for them to download a bunch of jars they don't need. And the opposite is true as well: if I only need the tool, why would I need to download 200Mb of server jars? |
|
With real (not in-memory) persistence the As to whether to keep a separate downloadable archive just for the admin tool, I suppose it may be useful only if the server runs in docker. However, in that case, it might be preferable to have a separate docker image for the admin tool and have only the combined archive-based distribution. WDYT? |
If that's the plan I can live with that 👍 |
What’s included in the PR:
TODOs:
The PR is technically ready, but I plan to wait until the 0.10 release is finalized to avoid disrupting the release process.