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

[draf] unify drop catalog and metalake #4883

Closed
wants to merge 1 commit into from

Conversation

mchades
Copy link
Contributor

@mchades mchades commented Sep 8, 2024

What changes were proposed in this pull request?

  • add in-use property to metalake and catalog
  • add in-use to drop catalog/metalake APIs
  • add inactivate() and activate() API for metalake and catalog
  • new definition for drop matalake/catalog

Why are the changes needed?

unify drop catalog and metalake

Does this PR introduce any user-facing change?

yes

How was this patch tested?

tests added

@mchades
Copy link
Contributor Author

mchades commented Sep 11, 2024

@jerryshao Could you please review the new API definition when you have time? Thank you.

* @throws NonEmptyEntityException If the catalog is not empty.
* @throws EntityInUseException If the catalog is in use.
*/
boolean dropCatalog(NameIdentifier ident) throws NonEmptyEntityException, EntityInUseException;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use String catalog name, not catalog name identifier.

*/
boolean dropCatalog(String catalogName);
void inactivateCatalog(NameIdentifier ident) throws NoSuchCatalogException;
Copy link
Contributor

Choose a reason for hiding this comment

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

deactivate, not inactivate.

@jerryshao
Copy link
Contributor

I need to think more about it, generally the API design LGTM.

@mchades
Copy link
Contributor Author

mchades commented Sep 27, 2024

I will close this PR, the rest of the tasks will be tracked by #5031

@mchades mchades closed this Sep 27, 2024
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.

2 participants