Skip to content

Conversation

@findepi
Copy link
Member

@findepi findepi commented Aug 8, 2024

No description provided.

@findepi findepi requested a review from a team as a code owner August 8, 2024 10:24
@findepi
Copy link
Member Author

findepi commented Aug 15, 2024

thank you @sfc-gh-emaynard @aihuaxu @eric-maynard @RussellSpitzer for your reviews!

i will rebase to resolve conflicts

@findepi findepi force-pushed the findepi/remove-unused-local-variables-1b3f38 branch 2 times, most recently from c0de2bb to afc61a3 Compare August 18, 2024 19:46
@findepi
Copy link
Member Author

findepi commented Aug 18, 2024

@sfc-gh-emaynard can you please approve the github workflow again?

@findepi
Copy link
Member Author

findepi commented Aug 19, 2024

thank you

@findepi findepi force-pushed the findepi/remove-unused-local-variables-1b3f38 branch from afc61a3 to 1524ad7 Compare August 23, 2024 18:33
@findepi findepi force-pushed the findepi/remove-unused-local-variables-1b3f38 branch from 1524ad7 to 9344121 Compare August 24, 2024 08:58
@findepi
Copy link
Member Author

findepi commented Aug 24, 2024

thank you @snazy @sfc-gh-ygu @flyrain for your comments!
please take another look!

Copy link
Contributor

@flyrain flyrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@snazy
Copy link
Member

snazy commented Aug 25, 2024

@findepi LGTM, just that GitHub complains that there are conflicts (but doesn't say in which files 🤷). Mind rebasing to get rid of this warning?

@findepi findepi force-pushed the findepi/remove-unused-local-variables-1b3f38 branch from 9344121 to 5ff930b Compare August 25, 2024 16:58
@findepi
Copy link
Member Author

findepi commented Aug 25, 2024

@snazy thanks, rebased!

@RussellSpitzer RussellSpitzer merged commit 29b8805 into apache:main Aug 26, 2024
@RussellSpitzer
Copy link
Member

Thanks @findepi and thanks @ebyhr , @sfc-gh-emaynard , @snazy , @flyrain and @aihuaxu for review

@findepi findepi deleted the findepi/remove-unused-local-variables-1b3f38 branch August 26, 2024 06:33
snazy added a commit to snazy/polaris that referenced this pull request Nov 20, 2025
* Avoid calling deprecated `TableMetadataParser.read(FileIO, InputFile)` method. (apache#2609)

Call `read(InputFile)` instead, as instructed by Iceberg javadoc.

* Add doc notes about EclipseLink removal (apache#2605)

* chore(docs): add polaris-api-specs section (apache#2598)

* docs(README): Updating the READMEs to Reflect the Project Structure (apache#2599)

* docs(README): Updating the READMEs to Reflect the Project Structure

* fix(deps): update dependency io.opentelemetry:opentelemetry-bom to v1.54.1 (apache#2613)

* Add Code of Conduct entry to the ASF menu (apache#2537)

* Use the ASF Code Of Conduct

* Update site/hugo.yaml

Co-authored-by: Robert Stupp <snazy@snazy.de>

---------

Co-authored-by: Robert Stupp <snazy@snazy.de>

* fix(deps): update dependency org.postgresql:postgresql to v42.7.8 (apache#2619)

* chore(deps): update dependency mypy to >=1.18, <=1.18.2 (apache#2617)

* Update registry.access.redhat.com/ubi9/openjdk-21-runtime Docker tag to v1.23-6.1758133907 (apache#2612)

* Introduce alternate in-memory buffering event listener (apache#2574)

* fix(deps): update dependency org.assertj:assertj-core to v3.27.5 (apache#2618)

* chore(deps): update dependency virtualenv to >=20.34.0,<20.35.0 (apache#2614)

* Add Community Meeting 20250918 (apache#2622)

* Add 1.1.0-incubating release on the website (apache#2621)

* Add 1.1.0-incubating release content (apache#2625)

* chore(errorprone): Enabling EqualsGetClass, PatternMatchingInstanceof, and UnusedMethod in ErrorProne (apache#2600)

* fix(deps): update dependency com.adobe.testing:s3mock-testcontainers to v4.9.1 (apache#2626)

* Unify create/loadTable call paths (apache#2589)

In preparation for implementing sending non-credential config
to REST Catalog clients for apache#2207 this PR unifies calls paths
for create/load table operations.

This change does not have any differences in authorization.

This change is not expecte to have any material behaviour
differences to the affected code paths.

The main idea is to consolidate decision-making for that
to include into REST responses and use method parameters
like `EnumSet<AccessDelegationMode> delegationModes` for
driving those decisions.

* Remove numeric identifier from PolarisPrincipal (apache#2388)

This change removes the requirement for Polaris principals to have a numeric identifier, by removing the only sites where such an identifier was required:

- In the `Resolver`. Instead, the `Resolver` now performs a lookup by principal name.
- In  `PolarisAdminService`. Instead, the code now compares the principal name against the entity name.

Note: the lookup in the `Resolver` is still necessary, because the `Resolver` also needs to fetch the grant records.

* Include principal name in Polaris tokens (apache#2389)

* Include principal name in Polaris tokens

Summary of changes:

- Instead of including the principal id twice in the token, the principal name is now used as the subject claim. While the default authenticator doesn't need the principal name and works with just the principal id, not having the "real" principal name available could be a problem for other authenticator implementations.

- `DecodedToken` has been refactored and renamed to `InternalPolarisCredential`. It is also now a package-private component.

- `TokenBroker.verify()` now returns PolarisCredential.

* rename to InternalPolarisToken

* main: bump to 1.2.0-incubating-SNAPSHOT (apache#2624)

* bump version.txt to 1.2.0-incubating-SNAPSHOT

* virtualenv: wider version range (apache#2623)

see apache#2614 (comment)

* Remove ActiveRolesProvider (apache#2390)

Summary of changes:

- As proposed on the ML, `ActiveRolesProvider` is removed, and `DefaultActiveRolesProvider` is merged into `DefaultAuthenticator`. `ActiveRolesAugmentor` is also merged into `AuthenticatingAugmentor`.

- The implicit convention that no roles in credentials == all roles requested is removed as it is ambiguous. Credentials must explicitly include the `PRINCIPAL_ROLE:ALL` pseudo-role to request all roles available.

- PersistedPolarisPrincipal is removed. It existed merely as a means of passing the `PrincipalEntity` from the authenticator to the roles provider. This is not necessary anymore.

* NoSQL: adaptions

* Last merged commit d1d359a

---------

Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com>
Co-authored-by: Artur Rakhmatulin <artur.rakhmatulin@gmail.com>
Co-authored-by: Adam Christian <105929021+adam-christian-software@users.noreply.github.com>
Co-authored-by: Mend Renovate <bot@renovateapp.com>
Co-authored-by: JB Onofré <jbonofre@apache.org>
Co-authored-by: Alexandre Dutra <adutra@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants