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

Fix javadoc in OpenAPI generated code #401

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Conversation

dimas-b
Copy link
Contributor

@dimas-b dimas-b commented Oct 23, 2024

Fix the mustache pattern to do HTML escaping for special chars in javadoc (do not use tripple {{{).

Original warning:

> Task :polaris-service:javadoc
/home/.../polaris-service/build/generated/src/main/java/org/apache/polaris/service/catalog/api/IcebergRestCatalogApi.java:489: warning: invalid input: '<'
   * Load a table from the catalog.  The response contains both configuration and table metadata. The configuration, if non-empty is used as additional configuration for the table that overrides catalog configuration. For example, this configuration may change the FileIO implementation to be used for the table.  The response also contains the table's full metadata, matching the table metadata JSON file.  The catalog configuration may contain credentials that should be used for subsequent requests for the table. The configuration key \"token\" is used to pass an access token to be used as a bearer token for table requests. Otherwise, a token may be passed using a RFC 8693 token type as a configuration key. For example, \"urn:ietf:params:oauth:token-type:jwt=<JWT-token>\".

@eric-maynard
Copy link
Contributor

Can we consider instead making this fix in the upstream Iceberg spec?

@flyrain
Copy link
Contributor

flyrain commented Oct 24, 2024

Can we consider instead making this fix in the upstream [Iceberg spec]?

We should do that, otherwise, the issue will come back next time Polaris syncs the Iceberg rest spec.

Fix the mustache pattern to do HTML escaping for special chars in javadoc (do not use tripple `{{{`).

Original warning:

```
> Task :polaris-service:javadoc
/home/.../polaris-service/build/generated/src/main/java/org/apache/polaris/service/catalog/api/IcebergRestCatalogApi.java:489: warning: invalid input: '<'
   * Load a table from the catalog.  The response contains both configuration and table metadata. The configuration, if non-empty is used as additional configuration for the table that overrides catalog configuration. For example, this configuration may change the FileIO implementation to be used for the table.  The response also contains the table's full metadata, matching the table metadata JSON file.  The catalog configuration may contain credentials that should be used for subsequent requests for the table. The configuration key \"token\" is used to pass an access token to be used as a bearer token for table requests. Otherwise, a token may be passed using a RFC 8693 token type as a configuration key. For example, \"urn:ietf:params:oauth:token-type:jwt=<JWT-token>\".
```
@dimas-b
Copy link
Contributor Author

dimas-b commented Oct 24, 2024

Upstreaming is not necessary. The pattern turned out to be local to Polaris, but it was using the unescaped way of including "notes".

sfc-gh-emaynard

This comment was marked as duplicate.

Copy link
Contributor

@eric-maynard eric-maynard left a comment

Choose a reason for hiding this comment

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

LGTM, good catch! Perhaps we should adjust the PR description / name based on the refactor.

@dimas-b
Copy link
Contributor Author

dimas-b commented Oct 24, 2024

The PR description has been updated, indeed.

@eric-maynard
Copy link
Contributor

Nice! I think since we aren't changing any javadoc anymore the title might still be misleading. Either way, LGTM

@dimas-b
Copy link
Contributor Author

dimas-b commented Oct 24, 2024

Generated javadoc is changed by virtue of changing the pattern that generates it.

@flyrain flyrain enabled auto-merge (squash) October 24, 2024 20:17
@flyrain flyrain merged commit 24cd8ec into apache:main Oct 24, 2024
5 checks passed
@dimas-b dimas-b deleted the fix-javadoc-warn branch October 25, 2024 15:12
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.

5 participants