lidavidm commented on a change in pull request #125:
URL: https://github.com/apache/arrow-cookbook/pull/125#discussion_r788773591
##########
File path: java/CONTRIBUTING.rst
##########
@@ -0,0 +1,38 @@
+Bulding the Java Cookbook
+=========================
+
+The Java cookbook uses the Sphinx documentation system.
+
+Running ``make java`` from the cookbook root directory (the one where
+the ``README.rst`` exists) will install all necessary dependencies
+and will compile the cookbook to HTML.
+
+You will see the compiled result inside the ``build/java`` directory.
+
+Testing Java Recipes
+====================
+
+All recipes in the cookbook must be tested. The cookbook uses
+``javadoctest`` to verify the recipes.
Review comment:
We should explain here that we need at least JDK9 for JShell.
##########
File path: java/ext/javadoctest.py
##########
@@ -0,0 +1,92 @@
+import os
+import pathlib
+import subprocess
+
+from sphinx.ext.doctest import (Any, Dict, DocTestBuilder, TestcodeDirective,
+ TestoutputDirective, doctest, sphinx)
+from sphinx.locale import __
+
+
+class JavaDocTestBuilder(DocTestBuilder):
+ """
+ Runs java test snippets in the documentation.
+ """
+
+ name = "javadoctest"
+ epilog = __(
+ "Java testing of doctests in the sources finished, look at the "
+ "results in %(outdir)s/output.txt."
+ )
+
+ def compile(
+ self, code: str, name: str, type: str, flags: Any, dont_inherit: bool
+ ) -> Any:
+ # go to project that contains all your arrow maven dependencies
+ path_arrow_project = os.path.join(pathlib.Path.cwd(), "source", "demo")
+
+ # create list of all arrow jar dependencies
+ subprocess.run(
Review comment:
```suggestion
subprocess.check_call(
```
##########
File path: java/ext/javadoctest.py
##########
@@ -0,0 +1,92 @@
+import os
+import pathlib
+import subprocess
+
+from sphinx.ext.doctest import (Any, Dict, DocTestBuilder, TestcodeDirective,
+ TestoutputDirective, doctest, sphinx)
+from sphinx.locale import __
+
+
+class JavaDocTestBuilder(DocTestBuilder):
+ """
+ Runs java test snippets in the documentation.
+ """
+
+ name = "javadoctest"
+ epilog = __(
+ "Java testing of doctests in the sources finished, look at the "
+ "results in %(outdir)s/output.txt."
+ )
+
+ def compile(
+ self, code: str, name: str, type: str, flags: Any, dont_inherit: bool
+ ) -> Any:
+ # go to project that contains all your arrow maven dependencies
+ path_arrow_project = os.path.join(pathlib.Path.cwd(), "source", "demo")
+
+ # create list of all arrow jar dependencies
+ subprocess.run(
+ [
+ "mvn",
+ "-q",
+ "dependency:build-classpath",
+ "-DincludeTypes=jar",
+ "-Dmdep.outputFile=.cp.tmp",
+ ],
+ cwd=path_arrow_project,
+ text=True,
+ )
+ if not os.path.exists(path_arrow_project + "/.cp.tmp"):
+ raise RuntimeError(
+ __("invalid process to create jshell dependencies library")
+ )
+
+ # get list of all arrow jar dependencies
+ with open(os.path.join(path_arrow_project, ".cp.tmp")) as f:
+ stdout_dependency = f.read()
+ if not stdout_dependency:
+ raise RuntimeError(
+ __("invalid process to list jshell dependencies library")
+ )
+
+ # execute java testing code thru jshell and read output
+ proc_java_arrow_code = subprocess.Popen(
+ ["echo", "" + code], stdout=subprocess.PIPE
+ )
+ proc_jshell_process = subprocess.Popen(
+ ["jshell", "--class-path", stdout_dependency, "-"],
+ stdin=proc_java_arrow_code.stdout,
+ stdout=subprocess.PIPE,
+ text=True,
+ )
+ proc_java_arrow_code.stdout.close()
+ out_java_arrow, err_java_arrow = proc_jshell_process.communicate()
Review comment:
Try this instead. We shouldn't need to use `echo` for this.
##########
File path: java/ext/javadoctest.py
##########
@@ -0,0 +1,92 @@
+import os
+import pathlib
+import subprocess
+
+from sphinx.ext.doctest import (Any, Dict, DocTestBuilder, TestcodeDirective,
+ TestoutputDirective, doctest, sphinx)
+from sphinx.locale import __
+
+
+class JavaDocTestBuilder(DocTestBuilder):
+ """
+ Runs java test snippets in the documentation.
+ """
+
+ name = "javadoctest"
+ epilog = __(
+ "Java testing of doctests in the sources finished, look at the "
+ "results in %(outdir)s/output.txt."
+ )
+
+ def compile(
+ self, code: str, name: str, type: str, flags: Any, dont_inherit: bool
+ ) -> Any:
+ # go to project that contains all your arrow maven dependencies
+ path_arrow_project = os.path.join(pathlib.Path.cwd(), "source", "demo")
+
+ # create list of all arrow jar dependencies
+ subprocess.run(
+ [
+ "mvn",
+ "-q",
+ "dependency:build-classpath",
+ "-DincludeTypes=jar",
+ "-Dmdep.outputFile=.cp.tmp",
+ ],
+ cwd=path_arrow_project,
+ text=True,
+ )
+ if not os.path.exists(path_arrow_project + "/.cp.tmp"):
+ raise RuntimeError(
+ __("invalid process to create jshell dependencies library")
+ )
+
+ # get list of all arrow jar dependencies
+ with open(os.path.join(path_arrow_project, ".cp.tmp")) as f:
+ stdout_dependency = f.read()
+ if not stdout_dependency:
+ raise RuntimeError(
+ __("invalid process to list jshell dependencies library")
+ )
+
+ # execute java testing code thru jshell and read output
+ proc_java_arrow_code = subprocess.Popen(
+ ["echo", "" + code], stdout=subprocess.PIPE
+ )
+ proc_jshell_process = subprocess.Popen(
+ ["jshell", "--class-path", stdout_dependency, "-"],
+ stdin=proc_java_arrow_code.stdout,
+ stdout=subprocess.PIPE,
+ text=True,
+ )
+ proc_java_arrow_code.stdout.close()
+ out_java_arrow, err_java_arrow = proc_jshell_process.communicate()
Review comment:
```suggestion
proc_jshell_process = subprocess.Popen(
["jshell", "--class-path", stdout_dependency, "-"],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
text=True,
)
out_java_arrow, err_java_arrow =
proc_jshell_process.communicate(code)
```
##########
File path: java/requirements.txt
##########
@@ -0,0 +1,3 @@
+black
Review comment:
This file should conventionally be named requirements-dev.txt, I believe.
We might want to pin versions for each of these? And add a note to the
README. (Sorry for the trouble here…)
##########
File path: java/ext/javadoctest.py
##########
@@ -0,0 +1,92 @@
+import os
+import pathlib
+import subprocess
+
+from sphinx.ext.doctest import (Any, Dict, DocTestBuilder, TestcodeDirective,
+ TestoutputDirective, doctest, sphinx)
+from sphinx.locale import __
+
+
+class JavaDocTestBuilder(DocTestBuilder):
+ """
+ Runs java test snippets in the documentation.
+ """
+
+ name = "javadoctest"
+ epilog = __(
+ "Java testing of doctests in the sources finished, look at the "
+ "results in %(outdir)s/output.txt."
+ )
+
+ def compile(
+ self, code: str, name: str, type: str, flags: Any, dont_inherit: bool
+ ) -> Any:
+ # go to project that contains all your arrow maven dependencies
+ path_arrow_project = os.path.join(pathlib.Path.cwd(), "source", "demo")
+
+ # create list of all arrow jar dependencies
+ subprocess.run(
Review comment:
My bad here, I forgot that only the `check_` variants raise an exception
if the process fails.
--
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.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
