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

[hotfix][docs] Fix Apache Avro Specification Link. #25769

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

donPain
Copy link

@donPain donPain commented Dec 9, 2024

Just correcting the url reference to access the Avro specification.

@flinkbot
Copy link
Collaborator

flinkbot commented Dec 9, 2024

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@davidradl
Copy link
Contributor

Please add [hotfix] at the start of the title as per the process when there is no associated Jira.
Please could you squash the commits.
Please could you do the Chinese version as well.

@donPain donPain changed the title [docs] Fix Apache Avro Specification Link. [hotfix][docs] Fix Apache Avro Specification Link. Dec 10, 2024
@donPain donPain force-pushed the patch-1 branch 4 times, most recently from 562cb43 to 378f7bf Compare December 10, 2024 13:32
@donPain
Copy link
Author

donPain commented Dec 10, 2024

@davidradl I made the changes, could you review it again?

@@ -218,4 +218,4 @@ So the following table lists the type mapping from Flink type to Avro type.

In addition to the types listed above, Flink supports reading/writing nullable types. Flink maps nullable types to Avro `union(something, null)`, where `something` is the Avro type converted from Flink type.

You can refer to [Avro Specification](https://avro.apache.org/docs/current/spec.html) for more information about Avro types.
You can refer to [Avro Specification](https://avro.apache.org/docs/++version++/specification/) for more information about Avro types.
Copy link
Contributor

Choose a reason for hiding this comment

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

looks good - I notice that there is. reference in parquet.md as well to the spec for 1.10. should we align that link here as well?

Copy link
Author

Choose a reason for hiding this comment

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

Good point, I changed all values ​​from version "1.10.0" to "++version++".

@snuyanzin
Copy link
Contributor

can you also change it at

* rules (@see <a href="https://avro.apache.org/docs/current/spec.html#Schema+Resolution">Schema

@donPain
Copy link
Author

donPain commented Dec 12, 2024

can you also change it at

* rules (@see <a href="https://avro.apache.org/docs/current/spec.html#Schema+Resolution">Schema

Sure, I made the changes.

@snuyanzin
Copy link
Contributor

run ./mvnw spotless:apply
before

git add . && git commit

as mentioned at https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=64325&view=logs&j=52b61abe-a3cc-5bde-cc35-1bbe89bb7df5&t=54421a62-0c80-5aad-3319-094ff69180bb&l=12927 (the reason of failure)

@donPain
Copy link
Author

donPain commented Dec 13, 2024

@flinkbot run azure

@davidradl
Copy link
Contributor

Reviewed by Chi on 12/12/24 Approve - looking for committer to merge

使用 JSON 定义 Avro schemas。你可以从 [Avro specification](https://avro.apache.org/docs/1.10.0/spec.html) 获取更多关于 Avro schemas 和类型的信息。
此示例使用了一个在 [official Avro tutorial](https://avro.apache.org/docs/1.10.0/gettingstartedjava.html) 中描述的示例相似的 Avro schema:
使用 JSON 定义 Avro schemas。你可以从 [Avro specification](https://avro.apache.org/docs/++version++/spec.html) 获取更多关于 Avro schemas 和类型的信息。
此示例使用了一个在 [official Avro tutorial](https://avro.apache.org/docs/++version++/gettingstartedjava.html) 中描述的示例相似的 Avro schema:
Copy link
Contributor

Choose a reason for hiding this comment

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

This link seems broken

@rmetzger
Copy link
Contributor

Flink is using Avro 1.11.4. Wouldn't it make more sense to link to the docs of that version, then add a note to the pom file where the avro version is defined to make sure folks update the docs when updating the avro version in the pom file?
Ref: https://github.com/apache/flink/blob/release-1.20/pom.xml#L149

Also, I believe our docs system supports variables -- it would be much easier to have one variable defining the avro version, instead of having this spread all over the place.

@donPain
Copy link
Author

donPain commented Dec 13, 2024

Flink is using Avro 1.11.4. Wouldn't it make more sense to link to the docs of that version, then add a note to the pom file where the avro version is defined to make sure folks update the docs when updating the avro version in the pom file? Ref: https://github.com/apache/flink/blob/release-1.20/pom.xml#L149

Also, I believe our docs system supports variables -- it would be much easier to have one variable defining the avro version, instead of having this spread all over the place.

Apparently most of the avro documentation has broken links above 1.13.0.

Could you give me a reference on how to use variables to set the url in the documentation?

@donPain
Copy link
Author

donPain commented Dec 16, 2024

@flinkbot run azure

@davidradl
Copy link
Contributor

@rmetzger @donPain I had a quick look at the docs, it seems that Hugo does allow templating which supports variables. I can see the Flink code referencing the predefined variables with {{ }}. I see {{< all_versions >}} which looks like a custom variable; but I am not sure how it is populated. I see code bringing in the different connectors. I have not yet found how we would want to add global variables like this.

@davidradl
Copy link
Contributor

@rmetzger as discussed at the Chi meeting are you ok to merge this? @donPain can you raise a separate issue to track the variable replacement improvement idea please.

Copy link
Contributor

@rmetzger rmetzger left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.
Let's intro the variable in a separate JIRA / PR.

@rmetzger rmetzger merged commit d61126a into apache:release-1.20 Dec 20, 2024
@rmetzger
Copy link
Contributor

@donPain could you raise a PR forward-porting the commit to the master branch as well? Thx

@donPain
Copy link
Author

donPain commented Dec 24, 2024

rmetzger

Sure, here it is: #25850

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