You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/04 23:00:23 UTC
[GitHub] [beam] robertwb opened a new pull request #11048: [BEAM-9433]
Create expansion service artifact for common Java IOs.
robertwb opened a new pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] chamikaramj commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388067898
##########
File path: sdks/python/apache_beam/io/external/kafka.py
##########
@@ -57,6 +58,10 @@
])
+def default_io_expansion_service():
+ return BeamJarExpansionService('sdks:java:io:expansion-service:shadowJar')
Review comment:
Probably add a comment here or in BeamJarExpansionService to clarify how this works.
IIUC,
(1) For Beam version 2.x.y, we'll download the released kafka Jar for version 2.x.y
(2) For Beam HEAD at version 2.(x+1).y.dev, we'll download the Beam Kafka Java jar for version 2.(x+1).y-SNAHSHOT
(3) We'll always cache downloaded jars (in a sub-directory of the current directory ?). Please make sure that caching work for both released and non-released Beam.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-598891273
Run Java PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388756189
##########
File path: sdks/python/apache_beam/runners/portability/job_server.py
##########
@@ -125,10 +124,6 @@ def local_temp_dir(self, **kwargs):
class JavaJarJobServer(SubprocessJobServer):
-
- MAVEN_REPOSITORY = 'https://repo.maven.apache.org/maven2/org/apache/beam'
- JAR_CACHE = os.path.expanduser("~/.apache_beam/cache")
Review comment:
These are vestigial. That cls is never JavaJarJobServer.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] chamikaramj commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388382088
##########
File path: sdks/python/apache_beam/io/external/kafka.py
##########
@@ -166,6 +171,8 @@ def __init__(
serialization.ByteArraySerializer'.
:param expansion_service: The address (host:port) of the ExpansionService.
"""
+ if expansion_service is None:
Review comment:
Prob. we don't need this since we default to default_io_expansion_service() below ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r389052734
##########
File path: sdks/python/apache_beam/io/external/kafka.py
##########
@@ -57,6 +58,10 @@
])
+def default_io_expansion_service():
+ return BeamJarExpansionService('sdks:java:io:expansion-service:shadowJar')
Review comment:
Sure, added some unit tests. This is also tested end-to-end in the cross language tests.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-596022507
Run Portable_Python PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-595989672
Run Portable_Python PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb merged pull request #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb merged pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] chamikaramj commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388385184
##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -82,9 +82,18 @@ def start(self):
port, = pick_port(None)
cmd = [arg.replace('{{PORT}}', str(port)) for arg in self._cmd]
endpoint = 'localhost:%s' % port
- _LOGGER.warning("Starting service with %s", str(cmd).replace("',", "'"))
+ _LOGGER.info("Starting service with %s", str(cmd).replace("',", "'"))
try:
- self._process = subprocess.Popen(cmd)
+ self._process = subprocess.Popen(
+ cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+
+ def log_stdout():
Review comment:
Please add a comment about this logging fix.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388758912
##########
File path: sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java
##########
@@ -404,7 +408,11 @@ public void close() throws Exception {
public static void main(String[] args) throws Exception {
int port = Integer.parseInt(args[0]);
System.out.println("Starting expansion service at localhost:" + port);
- Server server = ServerBuilder.forPort(port).addService(new ExpansionService()).build();
+ ExpansionService service = new ExpansionService();
+ for (Map.Entry<String, TransformProvider> entry : service.registeredTransforms.entrySet()) {
+ System.out.println("\t" + entry.getKey() + ": " + entry.getValue());
Review comment:
I used stdout as this is the main entry point (similar to the statement above).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-595989628
Run Java PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] chamikaramj commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388382885
##########
File path: sdks/python/apache_beam/io/external/kafka.py
##########
@@ -57,6 +58,10 @@
])
+def default_io_expansion_service():
+ return BeamJarExpansionService('sdks:java:io:expansion-service:shadowJar')
Review comment:
Can we add some unit tests for BeamJarExpansionService and JavaJarExpansionService ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] chamikaramj commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388065824
##########
File path: sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java
##########
@@ -404,7 +408,11 @@ public void close() throws Exception {
public static void main(String[] args) throws Exception {
int port = Integer.parseInt(args[0]);
System.out.println("Starting expansion service at localhost:" + port);
- Server server = ServerBuilder.forPort(port).addService(new ExpansionService()).build();
+ ExpansionService service = new ExpansionService();
+ for (Map.Entry<String, TransformProvider> entry : service.registeredTransforms.entrySet()) {
+ System.out.println("\t" + entry.getKey() + ": " + entry.getValue());
Review comment:
Prob. change to a log statement if you wish to keep this.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] chamikaramj commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388384480
##########
File path: sdks/python/apache_beam/runners/portability/job_server.py
##########
@@ -125,10 +124,6 @@ def local_temp_dir(self, **kwargs):
class JavaJarJobServer(SubprocessJobServer):
-
- MAVEN_REPOSITORY = 'https://repo.maven.apache.org/maven2/org/apache/beam'
- JAR_CACHE = os.path.expanduser("~/.apache_beam/cache")
Review comment:
Seems like this is used here ?
https://github.com/apache/beam/blob/b91560cc354da471e3de502aad78dd059997a3d0/sdks/python/apache_beam/utils/subprocess_server.py#L198
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-598315732
Run Portable_Python PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388753643
##########
File path: sdks/python/apache_beam/io/external/kafka.py
##########
@@ -166,6 +171,8 @@ def __init__(
serialization.ByteArraySerializer'.
:param expansion_service: The address (host:port) of the ExpansionService.
"""
+ if expansion_service is None:
Review comment:
Done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-596047381
Run Portable_Python PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11048: [BEAM-9433] Create
expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#issuecomment-599006983
Run Java PreCommit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11048:
[BEAM-9433] Create expansion service artifact for common Java IOs.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11048: [BEAM-9433] Create expansion service artifact for common Java IOs.
URL: https://github.com/apache/beam/pull/11048#discussion_r388758465
##########
File path: sdks/python/apache_beam/io/external/kafka.py
##########
@@ -57,6 +58,10 @@
])
+def default_io_expansion_service():
+ return BeamJarExpansionService('sdks:java:io:expansion-service:shadowJar')
Review comment:
This is documented in BeamJarExpansionService, though I've expanded it. We don't download SNAPSHOTs (yet)?
----------------------------------------------------------------
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
With regards,
Apache Git Services