MonkeyCanCode commented on PR #2027:
URL: https://github.com/apache/polaris/pull/2027#issuecomment-3085366562
> > > Don't get me wrong, but it seems the setup is quite opinionated. The
Makefile requires devs to use new things, that do not necessarily match their
daily workflows. For example, I do not have `brew` on my Linux machine, as I do
no need it. Many people use sdkman or rely on distributions' rpm or deb
packages.
> > > Certain things in this change however look useful. For example the
helm stuff, so I wouldn't mind to have that as a separate "helper Makefile",
but without the requirement for brew, jenv and a specific Java distribution.
> > > Minikube can just work, but people may have special settings. I recall
a bunch of "specialties" wrt minikube + podman + images published to minikube's
registry.
> > > The line between "non-commercial OSS use" and "commercial use" of
Docker (i.e. you work on something for your employer) is blurry.
> > > I don't mind this script in general, if it's helpful for some users,
but I'm not convinced that it's suitable for most users.
> >
> >
> > I think the intension here is not to promo everyone to use `brew` but
rather a quick command for people to get their setup ready if they have `brew`.
As you pointed out, there are other package managers out there and people have
their own preference of the tooling management. We can add other option to
support additional installation methods (apk/yum/apt etc.). Also, the Makefile
does checks if users have the tools installed, installation will be skipped if
the needed tools are already present. In this case, if a user doesn't want to
install `brew` and we only support `brew`, the makefile will just aborted said
`brew` is missing etc. thus unable to fulfill the additional needed
dependencies. Then to go around this, users will then be responsible to setup
the needed dependencies to be able to use the Makefile until we supported their
ideal package managements (e.g. some people really like to use golang to put
tools under a given directory...that is a not a common package manage
ment but some people do use this route. We can add support for those if that
are more preferred).
> > And yes, with the one I pushed last night, I had defined the needed
dependencies per target. This mean, if an user is only working on helm, it
won't ask user to setup jenv etc nor specific java distribution.
> > Then for the licensing concern with Docker, I don't see a problem with
switching to podman. However, I do think we should make it configurable as not
everyone work on Polaris from a corporate perspective (also, docker itself is
free, the one that required license is docker desktop which has the GUI. Our
current doc is installing docker desktop which is not free for large corp. That
being said, within a large corporate, an user can still installed docker on
his/her Linux machine without GUI to avoid the licensing concern).
> > For the minikube lifecycle management, this is mainly from @adutra
feedback earlier with `run.sh` script which was used for manage lifecycle of
`kind` cluster. IMO, learning how to use a tool is not hard but it does takes
time. Rather a simple start/stop/cleanup/load will be more human friendly for a
person who is not familiar the tool of selection. For the specific settings
around minikube, we can see if they can be generic enough to add to this
workflow, however, based on our current published doc, those are not there.
This doesn't mean it won't work out of box, instead, it will just mean for an
end-users who have special settings needed around minikube won't be using the
`start` option from the Makefile to bring up a Minikube as the one we currently
have is really simple and taking default settings.
> > I created this tool mainly to easy my workflow with Mac when working
with Polaris. But we can for sure extend it to make it suitable for more users
which are not using Mac or not using brew.
>
> I had add podman support and brew optional with the latest push, here is
the new CLI interface:
>
> ```
> ➜ polaris git:(makefile) make
>
> Usage:
> make <target>
>
> General
> help Display this help.
>
> Polaris Build
> build Build Polaris server, admin,
and container images
> build-server Build Polaris server and
container image
> build-admin Build Polaris admin and
container image
> build-spark-plugin-3.5-2.12 Build Spark plugin v3.5 with
Scala v2.12
> build-spark-plugin-3.5-2.13 Build Spark plugin v3.5 with
Scala v2.13
> build-cleanup Clean build artifacts
> spotless-apply Apply code formatting using
Spotless Gradle plugin.
>
> Helm
> helm-doc-generate Generate Helm chart
documentation
> helm-unittest Run Helm chart unittest
> helm-lint Run Helm chart lint check
>
> Minikube
> minikube-start-cluster Start the Minikube cluster
> minikube-stop-cluster Stop the Minikube cluster
> minikube-load-images Load local Docker images into
the Minikube cluster
> minikube-cleanup Cleanup the Minikube cluster
>
> Pre-commit
> pre-commit Run tasks for pre-commit
>
> Dependencies
> check-dependencies Check if all requested
dependencies are present
> install-dependencies-brew Install dependencies if not
present via Brew
> install-optional-dependencies-brew Install optional dependencies
if not present via Brew
> ```
>
> Here is a sample command in case if u want to play around with podman:
>
> ```
> # build images to local
> CONTAINER_TOOL=podman make build
> # build images to local and push to minikube
> CONTAINER_TOOL=podman make minikube-load-images
> ```
>
> There is one catch found with podman build, where podman will try to add
default registry (in this case docker.io as prefix to the image):
>
> ```
> docker.io/apache/polaris-admin-tool latest
30d60d73d7ba 51 seconds ago 505 MB
> docker.io/apache/polaris-admin-tool
1.1.0-incubating-SNAPSHOT 30d60d73d7ba 51 seconds ago 505 MB
> ```
>
> This is a good practice but it will break our existed workflow. I can look
into more (either update chart to use docker.io as well as update docker build
to include the registry or check if there is a way to even remove it in
podman...quick check shows later is not possible).
For the image lookup issue, it is actually a bug in
https://github.com/kubernetes/minikube/issues/19396. So the above instructions
are still correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
