-
Notifications
You must be signed in to change notification settings - Fork 332
Use Makefile to simplify setup and commands #2027
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
|
The script is likely a nice addition for macOS users who are using brew. |
So we can switch to podman if that is a concern (or make docker substitute-able via env variable). Docker is still free for individuals or even small companies. However, most of the docs we have currently are using docker as opposed to podman. And yes, as you called out, this will be great for Linux/Unix users (brew can also be installed on WSL for Windows users). This doesn't work for native Windows users who are not using WSL (most of our docs are not Windows friendly at the moment as well). What do you think? |
|
If users/developers find this |
For sure, we don't necessary need to change the doc to replace the ones with gradlew commands. The main purpose for this one is to easy the process for developers who are not familiar with those tools and run 1-2 short commands to get things going. I will be adding more use cases (such as minikube) later this week. Again, thanks for your feedback and feel free to drop me things that you think can be benefits for developers to perform without copying/paste length commands from various pages. |
|
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 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 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 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: Here is a sample command in case if u want to play around with podman: 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): 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 kubernetes/minikube#19396. So the above instructions are still correct. |
dimas-b
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.
@MonkeyCanCode : Would you mind adding README notes about its intended use and the relationship to Gradle?
Added. Thanks for review. |
dimas-b
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.
Thx, @MonkeyCanCode !
|
thanks for review @eric-maynard, @snazy , and @dimas-b . I will leave this one up for couple more days before merge it. Once merged (assuming we would like to proceed with this route), then i will move those testing commands into this top level Makefile as well for python client and python code base lint etc. @HonahX |
eric-maynard
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.
The makefile itself looks good -- I also think the addition to the README is helpful context.
I do wonder how many build systems we want to end up with -- we could do the same thing with a shell script launched from gradle, for example.
IMO the convenience we gain here is worth it, but perhaps we should update other docs to actually point users at the makefile, such as the getting-started docs.
@HonahX may also have thoughts on our growing use of make.
IMO, a Makefile acts as a smart central organizer, which is often better than relying only on Gradle or separate Bash scripts. While Gradle is great for Java development, it gets complicated when you try to make it do everything, like building Python packages or deploying Helm charts (do-able via some thrid-party plugins which may get EOL at any time or using plain CMD plugin then handle various stdout/stderr parsing etc). Doing these external tasks directly in Gradle may lead to complex and hard-to-manage code in Gradle. Also, if parts of the project, like Helm charts, move to their own separate locations, a custom Gradle setup for those parts would have to be copied or redone. A Makefile, being more general, makes automation flexible and reusable. We could do many things with a Bash script. But Makefiles offer built-in features that Bash doesn't easily provide, like handling task dependencies and running tasks at the same time with parallel execution. Recreating these in a Bash script for a big project would be a lot of extra work. |
* chore(deps): update dependency mypy to >=1.17, <=1.17.0 (apache#2114) * Spark 3.5.6 and Iceberg 1.9.1 (apache#1960) * Spark 3.5.6 and Iceberg 1.9.1 * Cleanup * Add `pathStyleAccess` to AwsStorageConfigInfo (apache#2012) * Add `pathStyleAccess` to AwsStorageConfigInfo This change allows configuring the "path-style" access mode in S3 clients (both in Polaris Servers and Iceberg REST Catalog API clients). This change is applicable both to AWS storage and to non-AWS S3-compatible storage (apache#1530). * Add TestFileIOFactory helper (apache#2105) * Add FileIOFactory.wrapExisting helper * fix(deps): update dependency gradle.plugin.org.jetbrains.gradle.plugin.idea-ext:gradle-idea-ext to v1.2 (apache#2125) * fix(deps): update dependency boto3 to v1.39.7 (apache#2124) * Abstract polaris-runtime-service tests for all persistence implementations (apache#2106) The NoSQL persistence implementation has to run the Iceberg table & view catalog plus the Polaris specific tests as well. Reusing existing tests is beneficial to avoid a lot of code duplcation. This change moves the actual tests to `Abstract*` classes and refactors the existing tests to extend those. The NoSQL persistence work extends the same `Abstract*` classes but runs with different Quarkus test profiles. * Add IMPLICIT authentication support to the CLI (apache#2121) PRs apache#1925 and apache#1912 were merged around the same time. This PR connects the two changes and enables the CLI to accept IMPLICIT authentication type. Since Hadoop federated catalogs rely purely on IMPLICIT authentication, the CLI parsing test has been updated to reflect the same. * feat(helm): Add support for external authentication (apache#2104) * fix(deps): update dependency org.apache.iceberg:iceberg-bom to v1.9.2 (apache#2126) * fix(deps): update quarkus platform and group to v3.24.4 (apache#2128) * fix(deps): update dependency boto3 to v1.39.8 (apache#2129) * fix(deps): update dependency io.smallrye.config:smallrye-config-core to v3.13.3 (apache#2130) * Add newIcebergCatalog helper (apache#2134) creation of `IcebergCatalog` instances was quite redundant as tests mostly use the same parameters most of the time. also remove an unused field in 2 other tests. * Add server and client support for the new generic table `baseLocation` field (apache#2122) * Use Makefile to simplify setup and commands (apache#2027) * Use Makefile to simplify setup and commands * Add targets for minikube state management * Add podman support and spark plugin build * Add version target * Update README.md for Makefile usage and relation to the project * Fix nit * Package polaris client as python package (apache#2049) * Package polaris client as python package * Package polaris client as python package * Change owner to spark when copying files from local into Dockerfile * CI: Address failure from accessing GH API (apache#2132) CI sometimes fails with this failure: ``` * What went wrong: Execution failed for task ':generatePomFileForMavenPublication'. > Unable to process url: https://api.github.com/repos/apache/polaris/contributors?per_page=1000 ``` The sometimes failing request fetches the list of contributors to be published in the "root" POM. Unauthorized GH API requests have an hourly(?) limit of 60 requests per source IP. Authorized requests have a much higher rate limit. We do have a GitHub token available in every CI run, which can be used in GH API requests. This change adds the `Authorization` header for the failing GH API request to leverage the higher rate limit and let CI not fail (that often). * fix(deps): update dependency com.nimbusds:nimbus-jose-jwt to v10.4 (apache#2139) * fix(deps): update dependency com.diffplug.spotless:spotless-plugin-gradle to v7.2.0 (apache#2142) * fix(deps): update dependency software.amazon.awssdk:bom to v2.32.4 (apache#2146) * fix(deps): update dependency org.xerial.snappy:snappy-java to v1.1.10.8 (apache#2138) * fix(deps): update dependency org.junit:junit-bom to v5.13.4 (apache#2147) * fix(deps): update dependency boto3 to v1.39.9 (apache#2137) * fix(deps): update dependency com.fasterxml.jackson:jackson-bom to v2.19.2 (apache#2136) * Python client: add support for endpoint, sts-endpoint, path-style-access (apache#2127) This change adds support for endpoint, sts-endpoint, path-style-access to the Polaris Python client. Amends apache#1913 and apache#2012 * Remove PolarisEntityManager.getCredentialCache (apache#2133) `PolarisEntityManager` itself is not using the `StorageCredentialCache` but just hands it out via `getCredentialCache`. the only caller of `getCredentialCache` is `FileIOUtil.refreshAccessConfig`, which in in turn is only called by `DefaultFileIOFactory` and `IcebergCatalog`. note that in a follow-up we will likely be able to remove `PolarisEntityManager` usage completely from `IcebergCatalog`. additional cleanups: - use `StorageCredentialCache` injection in tests (but we need to invalidate all entries on test start) - remove unused `UserSecretsManagerFactory` from `PolarisCallContextCatalogFactory` * chore(deps): update registry.access.redhat.com/ubi9/openjdk-21-runtime docker tag to v1.22-1.1752676419 (apache#2150) * fix(deps): update dependency com.diffplug.spotless:spotless-plugin-gradle to v7.2.1 (apache#2152) * fix(deps): update dependency boto3 to v1.39.10 (apache#2151) * chore: fix class reference in the javadoc of TableLikeEntity (apache#2157) * fix(deps): update dependency commons-codec:commons-codec to v1.19.0 (apache#2160) * fix(deps): update dependency boto3 to v1.39.11 (apache#2159) * Last merged commit 395459f --------- Co-authored-by: Mend Renovate <[email protected]> Co-authored-by: Yong Zheng <[email protected]> Co-authored-by: Dmitri Bourlatchkov <[email protected]> Co-authored-by: Christopher Lambert <[email protected]> Co-authored-by: Pooja Nilangekar <[email protected]> Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: Yun Zou <[email protected]>
This PR brings in a root-level Makefile to simplify and centralize how we set up our development environment and run common build tasks for Polaris.
The Problem I Noticed:
Right now, getting set up or performing routine builds often means jumping between different documentation pages to find various tool installations (like
helm,kubectl,jq,jenv,git,docker) and then copying lengthygradlewcommands. While it makes sense to have granular instructions for deep dives or specific contributions, having a single, straightforward entry point for common tasks could really smooth out onboarding for new folks and make our development flow much more efficient.My Proposal (and a Working Example):
I've put together this PR as a first pass, a working example, to kick off a discussion on whether this centralized Makefile approach is something we want to adopt. It shows how we can:
makecommand. (We can definitely refine this further, perhaps breaking it down into component-specific sets if we build this out.)gradlewcalls with concisemakecommands to build the server, admin, or both.maketargets.pre-committarget to automate routine tasks like Helm documentation generation and applying Spotless formatting.Beyond this PR: Other Potential Use Cases:
If we move forward with this approach, the Makefile could also become a central point for:
makecommand.make start,make stop) and orchestrating Helm deployments.What I'm Looking For:
I'd really appreciate your thoughts on this centralized Makefile idea. Does this feel like the right path for how we want to manage our developer experience? Any feedback on the overall concept or the specific examples here would be super helpful as we decide whether to build this out further.
Mailing discussion:
https://lists.apache.org/thread/vjnptg3prdps9slkt7rnvv7ldcwld88s
Sample interface and commands output:
Main interface:
Run pre-commit
Run build
Minikube state management