Skip to content
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

Core: Add FileIO tracker/closer to REST catalog #7487

Merged
merged 1 commit into from
May 16, 2023

Conversation

nastra
Copy link
Contributor

@nastra nastra commented May 1, 2023

No description provided.

@github-actions github-actions bot added the core label May 1, 2023
@nastra nastra requested a review from danielcweeks May 1, 2023 13:55
@nastra nastra force-pushed the fileio-cache-in-rest-catalog branch from c462e42 to 06a4b5b Compare May 1, 2023 13:58
@nastra nastra closed this May 1, 2023
@nastra nastra reopened this May 1, 2023
@nastra
Copy link
Contributor Author

nastra commented May 1, 2023

CI seems to fail due to Could not HEAD 'https://repo.maven.apache.org/maven2/io/netty/netty-codec-http/4.1.75.Final/netty-codec-http-4.1.75.Final.pom'. Received status code 502 from server: Gateway Error

@nastra nastra closed this May 1, 2023
@nastra nastra reopened this May 1, 2023
@nastra nastra closed this May 2, 2023
@nastra nastra reopened this May 2, 2023
@nastra nastra force-pushed the fileio-cache-in-rest-catalog branch from 06a4b5b to 8c101e9 Compare May 3, 2023 11:15
@nastra nastra changed the title Core: Add FileIO cache for REST catalog Core: Add FileIO tracking/closer to REST catalog May 3, 2023
@nastra nastra changed the title Core: Add FileIO tracking/closer to REST catalog Core: Add FileIO tracker/closer to REST catalog May 3, 2023
.removalListener(
(RemovalListener<TableOperations, FileIO>)
(ops, fileIO, cause) -> {
if (null != fileIO) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would fileIO be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the API contract of the RemovalListener marks keys/values as being @Nullable: void onRemoval(@Nullable K key, @Nullable V value, @NonNull RemovalCause cause);

Since we're not using weakValues() here the value shouldn't be null as indicated by the Javadoc, so this is rather to be defensive and avoid warnings in IntelliJ (we have the same warning a few lines above in the session cache btw)

@nastra nastra force-pushed the fileio-cache-in-rest-catalog branch from 8c101e9 to e014c43 Compare May 3, 2023 16:33
@nastra nastra force-pushed the fileio-cache-in-rest-catalog branch from e014c43 to 60cf024 Compare May 3, 2023 17:05
@nastra nastra requested a review from rdblue May 11, 2023 09:43
@rdblue rdblue added this to the Iceberg 1.3.0 milestone May 16, 2023
@nastra nastra force-pushed the fileio-cache-in-rest-catalog branch from 60cf024 to 803e990 Compare May 16, 2023 06:56
@rdblue rdblue merged commit d23b36c into apache:master May 16, 2023
41 checks passed
@rdblue
Copy link
Contributor

rdblue commented May 16, 2023

Thanks for fixing this, @nastra! Good to have it in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants