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

[Improvement] IcebergRESTServer should not use Gravitino iceberg catalog with REST backend #5153

Open
FANNG1 opened this issue Oct 16, 2024 · 4 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed ICEBERG-REST-SERVER improvement Improvements on everything

Comments

@FANNG1
Copy link
Contributor

FANNG1 commented Oct 16, 2024

What would you like to be improved?

This may cause deadlock, if IcebergRESTServer using the Gravitino Iceberg catalog with the same Iceberg REST address.

GravitinoBasedIcebergCatalogWrapperProvider.java#getIcebergTableOps()

   Catalog catalog = client.loadMetalake(gravitinoMetalake).loadCatalog(catalogName);
    Preconditions.checkArgument(
        "lakehouse-iceberg".equals(catalog.provider()),
        String.format("%s.%s is not iceberg catalog", gravitinoMetalake, catalogName));
   // should check catalog backend in catalog.properties()

How should we improve?

No response

@FANNG1 FANNG1 added improvement Improvements on everything 0.7.0 Release v0.7.0 good first issue Good for newcomers help wanted Extra attention is needed ICEBERG-REST-SERVER and removed 0.7.0 Release v0.7.0 labels Oct 16, 2024
@lsyulong
Copy link
Contributor

Hello, can this task be assigned to me? I'll give it a try

@FANNG1
Copy link
Contributor Author

FANNG1 commented Oct 17, 2024

@lsyulong sure, please go ahead

@lsyulong
Copy link
Contributor

GravitinoBasedIcebergCatalogWrapperProvider

@FANNG1 I cloned the latest code here again, but couldn't find this class(GravitinoBasedIcebergCatalogWrapperProvider). Did you change the name

@FANNG1
Copy link
Contributor Author

FANNG1 commented Oct 22, 2024

GravitinoBasedIcebergCatalogWrapperProvider

The latest class is DynamicIcebergConfigProvider#getIcebergCatalogConfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed ICEBERG-REST-SERVER improvement Improvements on everything
Projects
None yet
Development

No branches or pull requests

2 participants