You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/07/19 19:54:31 UTC

[GitHub] [iceberg] guykhazma opened a new pull request #1217: Site: Fixes

guykhazma opened a new pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217


   This PR includes some minor fixes to the documentation site:
   
   - Javadoc bug which makes the search form unusable - 
   When using JDK 11 the Javadoc is generated using HTML 5 and has a search bar, due to a bug some of the links redirect to invalid locations, for example:
   ```
   https://iceberg.apache.org/javadoc/0.9.0/undefined/org/apache/iceberg/Transaction.html
   ```
   instead of:
   ```
   https://iceberg.apache.org/javadoc/0.9.0/org/apache/iceberg/Transaction.html
   ```
   The bug was fixed manually for version 0.9.0 and for future release I have modified the gradle task to make the necessary changes according to the solution suggested [here](https://stackoverflow.com/questions/52326318/maven-javadoc-search-redirects-to-undefined-url/52603413#52603413).
   
   - Java API page changes:
     - Adding documentation for transactions support
     - Adding documentation for `iceberg-spark3`
   
   - Minor typo fixes


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457662580



##########
File path: site/docs/api.md
##########
@@ -99,6 +99,8 @@ Available operations to update a table are:
 
 ### Transactions
 
+`Table` also enables to commit multiple table operations at once.

Review comment:
       I think we should either remove this empty section or expand it a bit beyond what you have here. How about this?
   
   > Transactions are used to commit multiple table changes in a single atomic operation. A transaction is used to create individual operations using factory methods, like `newAppend`, just like working with a `Table`. But operations created by a transaction are committed as a group when `commitTransaction` is called.
   
   ```java
   Transaction t = table.newTrasaction();
   
   // commit operations to the transaction
   t.newDelete().deleteFromRowFilter(filter).commit();
   t.newAppend().appendFile(data).commit();
   
   // commit all the changes to the table
   t.commitTransaction();
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] guykhazma commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
guykhazma commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457676926



##########
File path: tasks.gradle
##########
@@ -25,6 +25,20 @@ task aggregateJavadoc(type: Javadoc) {
   source subprojects.javadoc.source
   destinationDir project.rootProject.file("site/docs/javadoc/${getJavadocVersion()}")
   classpath = project.rootProject.files(subprojects.javadoc.classpath)
+
+  final JAVADOC_FIX_SEARCH_STR = '\n\n' +
+          'getURLPrefix = function(ui) {\n' +
+          '    return \'\';\n' +
+          '};\n'
+
+  doLast {
+    // Fix bug with search
+    if (JavaVersion.current() >= JavaVersion.VERSION_11) {

Review comment:
       The search is actually very nice addition, we can remove this fix and fix each time manually




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] guykhazma commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
guykhazma commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457680763



##########
File path: site/docs/api.md
##########
@@ -99,6 +99,8 @@ Available operations to update a table are:
 
 ### Transactions
 
+`Table` also enables to commit multiple table operations at once.

Review comment:
       thanks for the suggestion. updated accordingly.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] guykhazma commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
guykhazma commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457681977



##########
File path: tasks.gradle
##########
@@ -25,6 +25,20 @@ task aggregateJavadoc(type: Javadoc) {
   source subprojects.javadoc.source
   destinationDir project.rootProject.file("site/docs/javadoc/${getJavadocVersion()}")
   classpath = project.rootProject.files(subprojects.javadoc.classpath)
+
+  final JAVADOC_FIX_SEARCH_STR = '\n\n' +
+          'getURLPrefix = function(ui) {\n' +
+          '    return \'\';\n' +
+          '};\n'
+
+  doLast {
+    // Fix bug with search
+    if (JavaVersion.current() >= JavaVersion.VERSION_11) {

Review comment:
       the search was added starting with Java 9 (as part of the move to HTML 5), it became default in Java 10 (see [here](http://openjdk.java.net/jeps/224))




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#issuecomment-661362531


   Looks great, I'll merge. Thanks for the contribution, @guykhazma!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457658522



##########
File path: tasks.gradle
##########
@@ -25,6 +25,20 @@ task aggregateJavadoc(type: Javadoc) {
   source subprojects.javadoc.source
   destinationDir project.rootProject.file("site/docs/javadoc/${getJavadocVersion()}")
   classpath = project.rootProject.files(subprojects.javadoc.classpath)
+
+  final JAVADOC_FIX_SEARCH_STR = '\n\n' +
+          'getURLPrefix = function(ui) {\n' +
+          '    return \'\';\n' +
+          '};\n'
+
+  doLast {
+    // Fix bug with search
+    if (JavaVersion.current() >= JavaVersion.VERSION_11) {

Review comment:
       Maybe we should just require Javadoc to be built with Java 8 instead?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457680593



##########
File path: tasks.gradle
##########
@@ -25,6 +25,20 @@ task aggregateJavadoc(type: Javadoc) {
   source subprojects.javadoc.source
   destinationDir project.rootProject.file("site/docs/javadoc/${getJavadocVersion()}")
   classpath = project.rootProject.files(subprojects.javadoc.classpath)
+
+  final JAVADOC_FIX_SEARCH_STR = '\n\n' +
+          'getURLPrefix = function(ui) {\n' +
+          '    return \'\';\n' +
+          '};\n'
+
+  doLast {
+    // Fix bug with search
+    if (JavaVersion.current() >= JavaVersion.VERSION_11) {

Review comment:
       Oh, the search is only available with Java 11? Then that's a nice reason to keep it as you have it here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] guykhazma commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
guykhazma commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457681977



##########
File path: tasks.gradle
##########
@@ -25,6 +25,20 @@ task aggregateJavadoc(type: Javadoc) {
   source subprojects.javadoc.source
   destinationDir project.rootProject.file("site/docs/javadoc/${getJavadocVersion()}")
   classpath = project.rootProject.files(subprojects.javadoc.classpath)
+
+  final JAVADOC_FIX_SEARCH_STR = '\n\n' +
+          'getURLPrefix = function(ui) {\n' +
+          '    return \'\';\n' +
+          '};\n'
+
+  doLast {
+    // Fix bug with search
+    if (JavaVersion.current() >= JavaVersion.VERSION_11) {

Review comment:
       the search was added starting with Java 9, it became default in Java 10 (see [here](http://openjdk.java.net/jeps/224))




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a change in pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217#discussion_r457662580



##########
File path: site/docs/api.md
##########
@@ -99,6 +99,8 @@ Available operations to update a table are:
 
 ### Transactions
 
+`Table` also enables to commit multiple table operations at once.

Review comment:
       I think we should either remove this empty section or expand it a bit beyond what you have here.
   
   Transactions are used to commit multiple table changes in a single atomic operation. A transaction is used to create individual operations using factory methods, like `newAppend`, just like working with a `Table`. But operations created by a transaction are committed as a group when `commitTransaction` is called.
   
   ```java
   Transaction t = table.newTrasaction();
   
   // commit operations to the transaction
   t.newDelete().deleteFromRowFilter(filter).commit();
   t.newAppend().appendFile(data).commit();
   
   // commit all the changes to the table
   t.commitTransaction();
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue merged pull request #1217: Site: Fixes

Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #1217:
URL: https://github.com/apache/iceberg/pull/1217


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org