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

[#6577] docs: fix param typo from 'type' to 'catalog_type' #6581

Merged

Conversation

AndreVale69
Copy link
Contributor

What changes were proposed in this pull request?

Fix parameter typo for create_catalog function in hadoop-catalog fileset documentation and manage-fileset and manage-model. Change parameter typo from type to catalog_type.

Why are the changes needed?

Incorrect parameter name in the documentation.

Fix: #6577

Does this PR introduce any user-facing change?

No.

How was this patch tested?

None.

@AndreVale69 AndreVale69 marked this pull request as ready for review March 3, 2025 13:39
Copy link
Contributor

@zhengkezhou1 zhengkezhou1 left a comment

Choose a reason for hiding this comment

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

Thanks! but unfortunately, I think we don't need this, as Catalog.Type is already clear enough.

@yuqi1129
Copy link
Contributor

yuqi1129 commented Mar 4, 2025

Thanks! but unfortunately, I think we don't need this, as Catalog.Type is already clear enough.

This sounds reasonable. Java API is as follows

  public Catalog createCatalog(
      String catalogName,
      Catalog.Type type,
      String provider,
      String comment,
      Map<String, String> properties)
      throws NoSuchMetalakeException, CatalogAlreadyExistsException {
    return getMetalake().createCatalog(catalogName, type, provider, comment, properties);
  }

Could you try to change python API and change the parameter name from catalog_type to type to make it consistent with JAVA API.

@AndreVale69
Copy link
Contributor Author

Thanks! but unfortunately, I think we don't need this, as Catalog.Type is already clear enough.

As @yuqi1129 suggested, I could try to modify the Python API to achieve consistency between the JAVA and Python APIs. This might be interesting for me since I'm a new contributor.

@jerryshao
Copy link
Contributor

I think the change itself is good, because it indeed fix the doc issue.

As for the name consistency between Java and Python, since we already use catalog_type as a parameter name in Python, the change will break the compatibility, it is not worth doing for now.

@jerryshao jerryshao added the branch-0.8 Automatically cherry-pick commit to branch-0.8 label Mar 7, 2025
Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerryshao jerryshao merged commit 392cdd5 into apache:main Mar 7, 2025
25 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 7, 2025
### What changes were proposed in this pull request?

Fix parameter typo for `create_catalog` function in `hadoop-catalog`
fileset documentation and `manage-fileset` and `manage-model`. Change
parameter typo from `type` to `catalog_type`.

### Why are the changes needed?

Incorrect parameter name in the documentation.

Fix: #6577

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

None.

Co-authored-by: Jerry Shao <[email protected]>
@AndreVale69 AndreVale69 deleted the docs/fix-param-typo-hadoop-catalog-6577 branch March 7, 2025 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-0.8 Automatically cherry-pick commit to branch-0.8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug report] Document bug in fileset python sample code
4 participants