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

[#3731] docs(API): Update the tutorial and document for the Java client API change #3742

Merged
merged 5 commits into from
Jul 8, 2024

Conversation

shaofengshi
Copy link
Contributor

What changes were proposed in this pull request?

Update the documentations for the latest Java API change

Why are the changes needed?

To make the document sync with API change.

Fix: #3731

Does this PR introduce any user-facing change?

No

How was this patch tested?

Document, local previewd.

@shaofengshi shaofengshi self-assigned this Jun 4, 2024
@shaofengshi shaofengshi marked this pull request as draft June 4, 2024 04:06
@shaofengshi shaofengshi marked this pull request as ready for review July 3, 2024 08:44
@shaofengshi shaofengshi requested a review from xunliu July 3, 2024 08:44
@yuqi1129
Copy link
Contributor

yuqi1129 commented Jul 4, 2024

@shaofengshi
Can you also modify the Java doc regarding interface changes? for example

  /**
   * Create a fileset metadata in the catalog.
   *
   * <p>If the type of the fileset object is "MANAGED", the underlying storageLocation can be null,
   * and Gravitino will manage the storage location based on the location of the schema.
   *
   * <p>If the type of the fileset object is "EXTERNAL", the underlying storageLocation must be set.
   *
   * @param ident A fileset identifier.
   * @param comment The comment of the fileset.
   * @param type The type of the fileset.
   * @param storageLocation The storage location of the fileset.
   * @param properties The properties of the fileset.
   * @return The created fileset metadata
   * @throws NoSuchSchemaException If the schema does not exist.
   * @throws FilesetAlreadyExistsException If the fileset already exists.
   */
  Fileset createFileset(
      NameIdentifier ident,
      String comment,
      Fileset.Type type,
      String storageLocation,
      Map<String, String> properties)
      throws NoSuchSchemaException, FilesetAlreadyExistsException;

The ident in method createFileset is different from before(before: {metalakeName}.{catalogName}.{schemaName}.{filesetName}, after:{schemaName}.{filesetName}) , the sentence "A fileset identifier" is very vague.

@shaofengshi
Copy link
Contributor Author

@shaofengshi Can you also modify the Java doc regarding interface changes? for example

  /**
   * Create a fileset metadata in the catalog.
   *
   * <p>If the type of the fileset object is "MANAGED", the underlying storageLocation can be null,
   * and Gravitino will manage the storage location based on the location of the schema.
   *
   * <p>If the type of the fileset object is "EXTERNAL", the underlying storageLocation must be set.
   *
   * @param ident A fileset identifier.
   * @param comment The comment of the fileset.
   * @param type The type of the fileset.
   * @param storageLocation The storage location of the fileset.
   * @param properties The properties of the fileset.
   * @return The created fileset metadata
   * @throws NoSuchSchemaException If the schema does not exist.
   * @throws FilesetAlreadyExistsException If the fileset already exists.
   */
  Fileset createFileset(
      NameIdentifier ident,
      String comment,
      Fileset.Type type,
      String storageLocation,
      Map<String, String> properties)
      throws NoSuchSchemaException, FilesetAlreadyExistsException;

The ident in method createFileset is different from before(before: {metalakeName}.{catalogName}.{schemaName}.{filesetName}, after:{schemaName}.{filesetName}) , the sentence "A fileset identifier" is very vague.

ok, I can update the java docs together in this PR

@shaofengshi
Copy link
Contributor Author

@shaofengshi Can you also modify the Java doc regarding interface changes? for example

  /**
   * Create a fileset metadata in the catalog.
   *
   * <p>If the type of the fileset object is "MANAGED", the underlying storageLocation can be null,
   * and Gravitino will manage the storage location based on the location of the schema.
   *
   * <p>If the type of the fileset object is "EXTERNAL", the underlying storageLocation must be set.
   *
   * @param ident A fileset identifier.
   * @param comment The comment of the fileset.
   * @param type The type of the fileset.
   * @param storageLocation The storage location of the fileset.
   * @param properties The properties of the fileset.
   * @return The created fileset metadata
   * @throws NoSuchSchemaException If the schema does not exist.
   * @throws FilesetAlreadyExistsException If the fileset already exists.
   */
  Fileset createFileset(
      NameIdentifier ident,
      String comment,
      Fileset.Type type,
      String storageLocation,
      Map<String, String> properties)
      throws NoSuchSchemaException, FilesetAlreadyExistsException;

The ident in method createFileset is different from before(before: {metalakeName}.{catalogName}.{schemaName}.{filesetName}, after:{schemaName}.{filesetName}) , the sentence "A fileset identifier" is very vague.

done, please review again, thanks!

@yuqi1129
Copy link
Contributor

yuqi1129 commented Jul 8, 2024

It's okay with me.

Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

LGTM

@yuqi1129 yuqi1129 merged commit ad35e8a into apache:main Jul 8, 2024
33 checks passed
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.

[Subtask] [Document] Update the tutorial and document for the Java client API change
2 participants