Use config-file to define errorprone rule, with some new rules #1233
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also enabled a couple more simple rules, and adding suppressions/fixes for/to the code. The two rules `EqualsGetClass` and `UnusedMethod`, which I think are useful, are not enabled yet, because that would mean actual code changes, which I do not want to do in this PR. The rule `PatternMatchingInstanceof`, introduced in apache#393, is disabled in this PR. It does not work before errorrpone 2.37.0 (via apache#1213) - requires additional changes to enable the rule (see apache#1215).
snazy
requested review from
adutra,
ashvina,
dennishuo,
dimas-b,
eric-maynard,
jackye1995,
jbonofre,
vvcephei,
collado-mike,
RussellSpitzer,
takidau,
MonkeyCanCode,
flyrain and
ebyhr
as code owners
|
Note: CI for #1213 would pass with this change. |
|
Any objections to these rules? |
dimas-b
reviewed
Apr 1, 2025
snazy
changed the title
dimas-b
approved these changes
Apr 4, 2025
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
github-project-automation
bot
moved this from Ready to merge
to Done
in Basic Kanban Board
|
The main after-merge errors. It may be related. |
|
Working on a fix for that |
|
@HonahX I still see the same; why does |
|
The project compiles fine in my env. after #1312 |
|
@eric-maynard Could you please share the error message you see? The main CI seems to pass too. |
|
It compiled fine on main for sure after @HonahX's fix. However, check this failed CI run on a dummy PR to see what I'm talking about. This is after running |
snazy
added a commit
to snazy/polaris
that referenced
this pull request
Apr 9, 2025
* Policy Store: PolicyMappingRecord with Persistence Impl (apache#1104) * Spark: Setup repository code structure and build (apache#1190) * Added freshness aware table loading using metadata file location for ETag (apache#1037) * Pulled in iceberg 1.8.0 spec changes for freshness aware table loading and added feature to Polaris * Changed etag support to use entityId:version tuple * fixed getresponse call * Changed etagged response to record and gave default implementation to ETaggableEntity * Made iceberg rest spec docs clearer * Added HTTP Compliant ETag and IfNoneMatch representations and separated persistence from etag logic * Changed ETag to be a record and improved semantics of IfNoneMatch * Fixed semantics of if none match * Removed ETag representation, consolidated in IfNoneMatch * fixed if none match parsing * Added table entity retrieval method to table operations * removed accidental commit of pycache folders * Fixed formatting * Changed to use metadata location hash * Ran formatting * use sha256 * Moved out ETag functions to utility class and removed ETaggedLoadTableResponse * Addressed comments * Fixed IcebergTableLikeEntity package rename * main: Update dependency io.opentelemetry.semconv:opentelemetry-semconv to v1.31.0 (apache#1288) * Update LICENSE and NOTICE in the distributions (admin and server) (apache#1258) * Gradle/Quarkus: make imageBuild task depend on jandex (apache#1290) * Core: Clarify the atomicity of BasePersistence methods (apache#1274) * Implement GenericTableCatalogAdapter (apache#1264) * rebase * more fixes * autolint * working on tests * stable test * autolint * polish * changes per review * some changes per review * grants * autolint * changes per review * changes per review * typofix * Improve code-containment and efficiency of etag-aware loading (apache#1296) * Improve code-containment and efficiency of etag-aware loading -Make the hash generation resilient against null metadataLocation -Use getResolvedPath instead of getPassthroughResolvedPath to avoid redundant persistence round-trip -Only try to calculate the etag for comparison against ifNoneMatch if the ifNoneMatch is actually provided * Add strict null-checking at callsites to generateETag, disallow passing null to generator * Add TODO to refactor shared logic for etag generation * Core: Add Endpoints and resource paths for Generic Table (apache#1286) * main: Update dependency com.nimbusds:nimbus-jose-jwt to v10.1 (apache#1299) * [JDBC] Part1 : ADD SQL script for Polaris setup (apache#1276) * main: Update registry.access.redhat.com/ubi9/openjdk-21-runtime Docker tag to v1.22-1.1743605859 (apache#1300) * done (apache#1297) * Add Polaris Community Meeting for April 3, 2025 (apache#1304) * Use config-file to define errorprone rule (apache#1233) Also enabled a couple more simple rules, and adding suppressions/fixes for/to the code. The two rules `EqualsGetClass` and `UnusedMethod`, which I think are useful, are not enabled yet, because that would mean actual code changes, which I do not want to do in this PR. The rule `PatternMatchingInstanceof`, introduced in apache#393, is disabled in this PR. It does not work before errorrpone 2.37.0 (via apache#1213) - requires additional changes to enable the rule (see apache#1215). * Add Yun as a contributor (apache#1310) * Refactor CatalogHandler to comply with ErrorProne rules (apache#1312) Fix the CI error after apache#1233 * Implement PolicyCatalog Stage 1: CRUD + ListPolicies (apache#1294) * main: Update dependency io.opentelemetry:opentelemetry-bom to v1.49.0 (apache#1316) * main: Update docker.io/jaegertracing/all-in-one Docker tag to v1.68.0 (apache#1317) * main: Update dependency boto3 to v1.37.28 (apache#1328) * main: Update dependency software.amazon.awssdk:bom to v2.31.16 (apache#1329) * Make `BasePolaritsMetaStoreManagerTest` and `(Base)ResolverTest` reusable (apache#1308) Moves the test cases into the `Base*` classes and make sure the classes can be reused by other persistence implementations. * main: Update dependency io.opentelemetry.semconv:opentelemetry-semconv to v1.32.0 (apache#1293) * main: Update mockito monorepo to v5.17.0 (apache#1311) * PySpark Update AWS Region (apache#1302) Co-authored-by: Travis Bowen <[email protected]> * main: Update dependency com.nimbusds:nimbus-jose-jwt to v10.2 (apache#1334) * main: Update dependency com.diffplug.spotless:spotless-plugin-gradle to v7.0.3 (apache#1335) * Maven publication: Produce correct `<scm><tag>` in `pom.xml` (apache#1330) `project.scm.tag` in a Maven pom is intended to refer to the SCM (Git) tag. We currently publish `main`, which is incorrect. This change omits the SCM tag for snapshot builds, but emits the Git tag for releases. * Remove `@StaticInitSafe` annotation (apache#1331) There was an issue around mapped configurations having the `@StaticInitSafe` annotation that led to _two_ instances (a "static" one and a "somewhet application-scoped" one) - this was fixed in Quarkus 3.21. One bug in smallrye-config is fixed for Quarkus > 3.21.0, another issue however remains. Since `@StaticInitSafe` annotated configs seem to cause some weird issues, it seems legit to remote that annotation altogether. This approach was [taken in Nessie](projectnessie/nessie#10606) as well. Investigations (via practical experiments) have proven that there's no measurable impact (runtime + heap) when doing this - and that's also been confirmed by Quarkus + Smallrye-config maintainers. Hence this change remotes that annotation from the code base. * Build/Release: Add a "generate digest" task and use for source tarball and Quarkus distributables (apache#1271) * Ensure that digest and signature are generated for both Polaris-Server and admin tar/zip distribution * Move "generate digest" functionality to a Gradle task * main: Update dependency com.google.errorprone:error_prone_core to v2.37.0 (apache#1213) * main: Update Quarkus Platform and Group to v3.21.1 (apache#1291) * main: Update dependency io.netty:netty-codec-http2 to v4.2.0.Final (apache#1301) * Adoptions after merge --------- Co-authored-by: Honah (Jonas) J. <[email protected]> Co-authored-by: gh-yzou <[email protected]> Co-authored-by: Mansehaj Singh <[email protected]> Co-authored-by: Mend Renovate <[email protected]> Co-authored-by: JB Onofré <[email protected]> Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: Yufei Gu <[email protected]> Co-authored-by: Eric Maynard <[email protected]> Co-authored-by: Dennis Huo <[email protected]> Co-authored-by: Prashant Singh <[email protected]> Co-authored-by: Travis Bowen <[email protected]> Co-authored-by: Travis Bowen <[email protected]>
snazy
added a commit
to snazy/polaris
that referenced
this pull request
Apr 11, 2025
…rvers` (#53) * Policy Store: PolicyMappingRecord with Persistence Impl (apache#1104) * Spark: Setup repository code structure and build (apache#1190) * Added freshness aware table loading using metadata file location for ETag (apache#1037) * Pulled in iceberg 1.8.0 spec changes for freshness aware table loading and added feature to Polaris * Changed etag support to use entityId:version tuple * fixed getresponse call * Changed etagged response to record and gave default implementation to ETaggableEntity * Made iceberg rest spec docs clearer * Added HTTP Compliant ETag and IfNoneMatch representations and separated persistence from etag logic * Changed ETag to be a record and improved semantics of IfNoneMatch * Fixed semantics of if none match * Removed ETag representation, consolidated in IfNoneMatch * fixed if none match parsing * Added table entity retrieval method to table operations * removed accidental commit of pycache folders * Fixed formatting * Changed to use metadata location hash * Ran formatting * use sha256 * Moved out ETag functions to utility class and removed ETaggedLoadTableResponse * Addressed comments * Fixed IcebergTableLikeEntity package rename * main: Update dependency io.opentelemetry.semconv:opentelemetry-semconv to v1.31.0 (apache#1288) * Update LICENSE and NOTICE in the distributions (admin and server) (apache#1258) * Gradle/Quarkus: make imageBuild task depend on jandex (apache#1290) * Core: Clarify the atomicity of BasePersistence methods (apache#1274) * Implement GenericTableCatalogAdapter (apache#1264) * rebase * more fixes * autolint * working on tests * stable test * autolint * polish * changes per review * some changes per review * grants * autolint * changes per review * changes per review * typofix * Improve code-containment and efficiency of etag-aware loading (apache#1296) * Improve code-containment and efficiency of etag-aware loading -Make the hash generation resilient against null metadataLocation -Use getResolvedPath instead of getPassthroughResolvedPath to avoid redundant persistence round-trip -Only try to calculate the etag for comparison against ifNoneMatch if the ifNoneMatch is actually provided * Add strict null-checking at callsites to generateETag, disallow passing null to generator * Add TODO to refactor shared logic for etag generation * Core: Add Endpoints and resource paths for Generic Table (apache#1286) * main: Update dependency com.nimbusds:nimbus-jose-jwt to v10.1 (apache#1299) * [JDBC] Part1 : ADD SQL script for Polaris setup (apache#1276) * main: Update registry.access.redhat.com/ubi9/openjdk-21-runtime Docker tag to v1.22-1.1743605859 (apache#1300) * done (apache#1297) * Add Polaris Community Meeting for April 3, 2025 (apache#1304) * Use config-file to define errorprone rule (apache#1233) Also enabled a couple more simple rules, and adding suppressions/fixes for/to the code. The two rules `EqualsGetClass` and `UnusedMethod`, which I think are useful, are not enabled yet, because that would mean actual code changes, which I do not want to do in this PR. The rule `PatternMatchingInstanceof`, introduced in apache#393, is disabled in this PR. It does not work before errorrpone 2.37.0 (via apache#1213) - requires additional changes to enable the rule (see apache#1215). * Add Yun as a contributor (apache#1310) * Refactor CatalogHandler to comply with ErrorProne rules (apache#1312) Fix the CI error after apache#1233 * Implement PolicyCatalog Stage 1: CRUD + ListPolicies (apache#1294) * main: Update dependency io.opentelemetry:opentelemetry-bom to v1.49.0 (apache#1316) * main: Update docker.io/jaegertracing/all-in-one Docker tag to v1.68.0 (apache#1317) * main: Update dependency boto3 to v1.37.28 (apache#1328) * main: Update dependency software.amazon.awssdk:bom to v2.31.16 (apache#1329) * Make `BasePolaritsMetaStoreManagerTest` and `(Base)ResolverTest` reusable (apache#1308) Moves the test cases into the `Base*` classes and make sure the classes can be reused by other persistence implementations. * main: Update dependency io.opentelemetry.semconv:opentelemetry-semconv to v1.32.0 (apache#1293) * main: Update mockito monorepo to v5.17.0 (apache#1311) * PySpark Update AWS Region (apache#1302) Co-authored-by: Travis Bowen <[email protected]> * main: Update dependency com.nimbusds:nimbus-jose-jwt to v10.2 (apache#1334) * main: Update dependency com.diffplug.spotless:spotless-plugin-gradle to v7.0.3 (apache#1335) * Maven publication: Produce correct `<scm><tag>` in `pom.xml` (apache#1330) `project.scm.tag` in a Maven pom is intended to refer to the SCM (Git) tag. We currently publish `main`, which is incorrect. This change omits the SCM tag for snapshot builds, but emits the Git tag for releases. * Remove `@StaticInitSafe` annotation (apache#1331) There was an issue around mapped configurations having the `@StaticInitSafe` annotation that led to _two_ instances (a "static" one and a "somewhet application-scoped" one) - this was fixed in Quarkus 3.21. One bug in smallrye-config is fixed for Quarkus > 3.21.0, another issue however remains. Since `@StaticInitSafe` annotated configs seem to cause some weird issues, it seems legit to remote that annotation altogether. This approach was [taken in Nessie](projectnessie/nessie#10606) as well. Investigations (via practical experiments) have proven that there's no measurable impact (runtime + heap) when doing this - and that's also been confirmed by Quarkus + Smallrye-config maintainers. Hence this change remotes that annotation from the code base. * Build/Release: Add a "generate digest" task and use for source tarball and Quarkus distributables (apache#1271) * Ensure that digest and signature are generated for both Polaris-Server and admin tar/zip distribution * Move "generate digest" functionality to a Gradle task * main: Update dependency com.google.errorprone:error_prone_core to v2.37.0 (apache#1213) * main: Update Quarkus Platform and Group to v3.21.1 (apache#1291) * main: Update dependency io.netty:netty-codec-http2 to v4.2.0.Final (apache#1301) * Remove unnecessary `clean` and `--no-build-cache` from Gradle invocations (apache#1338) `quarkusAppPartsBuild --rerun` is the right way to force a Docker image build. * Generalize bootstrapping in servers (apache#1313) * Remove `instanceof` checks from `QuarkusProducers`. * Remove the now unused `onStartup` method from `InMemoryPolarisMetaStoreManagerFactory`. * Instead, call the good old `bootstrapRealms` method from `QuarkusProducers`. * Add new config property to control which MetaStore types are bootstrapped automatically (defaults to `in-memory` as before). * There is no bootstrap behaviour change in this PR, only refactorings to simplify code. * Add info log message to indicate when a realm is bootstrapped in runtime using preset credentials. Future enhancements may include pulling preset credentials from a secret manager like Vault for bootstrapping (s discussed in comments on apache#1228). * main: Update actions/stale digest to 816d9db (apache#1341) * main: Update dependency com.adobe.testing:s3mock-testcontainers to v4 (apache#1342) * Revert "Introduce MetaStoreManagerFactory.initializeForService" This reverts commit 23abc6e. * NoSQL: Adopt to change in MSMF --------- Co-authored-by: Honah (Jonas) J. <[email protected]> Co-authored-by: gh-yzou <[email protected]> Co-authored-by: Mansehaj Singh <[email protected]> Co-authored-by: Mend Renovate <[email protected]> Co-authored-by: JB Onofré <[email protected]> Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: Yufei Gu <[email protected]> Co-authored-by: Eric Maynard <[email protected]> Co-authored-by: Dennis Huo <[email protected]> Co-authored-by: Prashant Singh <[email protected]> Co-authored-by: Travis Bowen <[email protected]> Co-authored-by: Travis Bowen <[email protected]> Co-authored-by: Dmitri Bourlatchkov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also enabled a couple more simple rules, and adding suppressions/fixes for/to the code.
The two rules
EqualsGetClassandUnusedMethod, which I think are useful, are not enabled yet, because that would mean actual code changes, which I do not want to do in this PR.The rule
PatternMatchingInstanceof, introduced in #393, is disabled in this PR. It does not work before errorrpone 2.37.0 (via #1213) - requires additional changes to enable the rule (see #1215).Also add a bunch of safety-nets via errorprone.