You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/24 05:18:14 UTC
[GitHub] [beam] leiyiz opened a new pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
leiyiz opened a new pull request #12365:
URL: https://github.com/apache/beam/pull/12365
changed parser and serialization code to use the same json format to represent models, added auction_price model and corrected the behavior of query 0 - 2 to align with the nexmark specification. created field_name file to map the fieldnames with string literals in json representation. created nexmark_query_util to put transforms that gets reused across different queries.
------------------------
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`).
- [x] 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 | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.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
[GitHub] [beam] pabloem commented on pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12365:
URL: https://github.com/apache/beam/pull/12365#issuecomment-668225000
oops sorry what happened in this PR after all?
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460402749
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/nexmark_model.py
##########
@@ -26,27 +26,25 @@
- The bid on an item for auction (Bid).
"""
+import json
class Person(object):
"Author of an auction or a bid."
def __init__(
- self, id, name, email, credit_card, city, state, timestamp, extra=None):
+ self, id, name, email, credit_card, city, state, date_time, extra=None):
self.id = id
self.name = name
- self.email = email # key
- self.credit_card = credit_card
+ self.emailAddress = email # key
Review comment:
I did that because
```python
json.dumps(self.__dict__)
```
outputs fieldnames that is the same with the name of fields so doing camel case sort of lines up with java version. I'll try to look for ways to change the names
----------------------------------------------------------------
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
[GitHub] [beam] y1chi commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
y1chi commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460201317
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/auction_price.py
##########
@@ -0,0 +1,28 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Result of Query2."""
+import json
+
+
+class AuctionPrice(object):
Review comment:
is this needed, why not just use tuple for the result?
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/queries/nexmark_query_util.py
##########
@@ -0,0 +1,36 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Utilities for working with NEXmark data stream."""
+
+
+import apache_beam as beam
+from apache_beam.testing.benchmarks.nexmark.models import nexmark_model
+
+
+def is_bid(event):
+ return isinstance(event, nexmark_model.Bid)
+
+
+class ToString(beam.PTransform):
Review comment:
I think this is pretty redundant, and we can just use beam.Map(repr) or beam.Map(str) in any pipelines.
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/queries/nexmark_query_util.py
##########
@@ -0,0 +1,36 @@
+#
Review comment:
why not put the helpers in nexmark_util? one util file should be good enough to hold all related helper methods/classes.
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/nexmark_model.py
##########
@@ -26,27 +26,25 @@
- The bid on an item for auction (Bid).
"""
+import json
class Person(object):
"Author of an auction or a bid."
def __init__(
- self, id, name, email, credit_card, city, state, timestamp, extra=None):
+ self, id, name, email, credit_card, city, state, date_time, extra=None):
self.id = id
self.name = name
- self.email = email # key
- self.credit_card = credit_card
+ self.emailAddress = email # key
Review comment:
python's style convention uses snake case for fields, so this is normally self.email_address, ditto for below
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/field_name.py
##########
@@ -0,0 +1,40 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+""" Field names for de-serializing json representation of Models
+"""
+
+
+class FieldName:
Review comment:
nit: FieldNames or Fields.
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on pull request #12365:
URL: https://github.com/apache/beam/pull/12365#issuecomment-668277274
> oops sorry what happened in this PR after all?
I closed this draft and had opened a PR here: https://github.com/apache/beam/pull/12427
----------------------------------------------------------------
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
[GitHub] [beam] y1chi commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
y1chi commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460176263
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/field_name.py
##########
@@ -0,0 +1,40 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+""" Field names for de-serializing json representation of Models
+"""
+
+
+class FieldName:
+ id = 'id'
Review comment:
constants normally uses all capitals, like ID, EMAIL_ADDRESS, ITEM_NAME etc.
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on pull request #12365:
URL: https://github.com/apache/beam/pull/12365#issuecomment-663351585
`R: @pabloem `
`R: @y1chi `
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz closed pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz closed pull request #12365:
URL: https://github.com/apache/beam/pull/12365
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460402907
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/queries/nexmark_query_util.py
##########
@@ -0,0 +1,36 @@
+#
Review comment:
My thinking was `nexmark_query_util.py` would only contain methods that are used in queries but certainly we can change them and put them into `nexmark_util.py`
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on pull request #12365:
URL: https://github.com/apache/beam/pull/12365#issuecomment-663651175
retest this please
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460403027
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/auction_price.py
##########
@@ -0,0 +1,28 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Result of Query2."""
+import json
+
+
+class AuctionPrice(object):
Review comment:
hmm I never thought about tuple but auctionPrice would allow me to define `__repr__` method to serialize to json
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460193581
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/field_name.py
##########
@@ -0,0 +1,40 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+""" Field names for de-serializing json representation of Models
+"""
+
+
+class FieldName:
+ id = 'id'
Review comment:
I have changed them
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz edited a comment on pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz edited a comment on pull request #12365:
URL: https://github.com/apache/beam/pull/12365#issuecomment-663351585
R: @pabloem
R: @y1chi
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460403119
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/queries/nexmark_query_util.py
##########
@@ -0,0 +1,36 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Utilities for working with NEXmark data stream."""
+
+
+import apache_beam as beam
+from apache_beam.testing.benchmarks.nexmark.models import nexmark_model
+
+
+def is_bid(event):
+ return isinstance(event, nexmark_model.Bid)
+
+
+class ToString(beam.PTransform):
Review comment:
sure, I can change it to use
```python
beam.Map(repr)
```
----------------------------------------------------------------
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
[GitHub] [beam] leiyiz commented on a change in pull request #12365: [BEAM-8258] python nexmark query 0-2 improvements
Posted by GitBox <gi...@apache.org>.
leiyiz commented on a change in pull request #12365:
URL: https://github.com/apache/beam/pull/12365#discussion_r460176936
##########
File path: sdks/python/apache_beam/testing/benchmarks/nexmark/models/field_name.py
##########
@@ -0,0 +1,40 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+""" Field names for de-serializing json representation of Models
+"""
+
+
+class FieldName:
+ id = 'id'
Review comment:
I see I see, I can change that definitely
----------------------------------------------------------------
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