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/05/16 17:49:35 UTC
[GitHub] [beam] iht opened a new pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
iht opened a new pull request #11731:
URL: https://github.com/apache/beam/pull/11731
The two Python katas about windowing fail because the timestamps for the elements are calculated based on the local timezone, and my timezone does not match the timezone hardcoded in the tests, and because parsing from strings using `fromisoformat` was failing.
In the first Kata, the timestamp was calculated from time objects, and converted to a timestamp in the local timezone. Thus, the results of the test depended on the configuration of the local timezone in the running system.
The tests were hardcoded with a timezone different to mine, and thus I always failed to pass this Kata. The changes in this commit change the type in `Event` to be a `datetime`, the timestamps are set in UTC, and the output in the tests is hardcoded in UTC too. This should ensure that the kata works regardless the timezone configured in the system running the kata.
In the second Kata, the code was failing with the following error:
```AttributeError: type object 'datetime.datetime' has no attribute 'fromisoformat'```
I changed the timestamps to be set using the `datetime` constructor, rather than parsing from strings using `fromisoformat`.
Both katas now pass with the examples hardcoded in the corresponding tests.
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_ValidatesRunner_Flink_Java11/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
[GitHub] [beam] iht commented on pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
iht commented on pull request #11731:
URL: https://github.com/apache/beam/pull/11731#issuecomment-630401560
Thank you!
----------------------------------------------------------------
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] iht commented on a change in pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
iht commented on a change in pull request #11731:
URL: https://github.com/apache/beam/pull/11731#discussion_r426241264
##########
File path: learning/katas/python/Windowing/Adding Timestamp/ParDo/task.py
##########
@@ -24,30 +25,30 @@
class Event:
- def __init__(self, id, event, date):
+ def __init__(self, id, event, timestamp):
self.id = id
self.event = event
- self.date = date
+ self.timestamp = timestamp
Review comment:
Fixed
----------------------------------------------------------------
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] iht commented on a change in pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
iht commented on a change in pull request #11731:
URL: https://github.com/apache/beam/pull/11731#discussion_r426235990
##########
File path: learning/katas/python/Windowing/Adding Timestamp/ParDo/task-info.yaml
##########
@@ -22,10 +22,10 @@ files:
- name: task.py
visible: true
placeholders:
- - offset: 1211
- length: 163
+ - offset: 1243
Review comment:
I have updated the placeholder values, and I have checked the course using Course preview. I think the placeholders are now correct.
----------------------------------------------------------------
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] iht commented on a change in pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
iht commented on a change in pull request #11731:
URL: https://github.com/apache/beam/pull/11731#discussion_r426241932
##########
File path: learning/katas/python/Windowing/Fixed Time Window/Fixed Time Window/task-info.yaml
##########
@@ -22,7 +22,7 @@ files:
- name: task.py
visible: true
placeholders:
- - offset: 2074
+ - offset: 2067
Review comment:
I am checking the boundaries of the placeholder in PyCharm, and I see it correctly
Maybe you were checking the previous placeholder box? I see that the box is starting 7 positions to the right (2074, after the `Wi`), and the placeholder I am submitting is starting 7 positions to the left (2067).
I have removed all the Placeholders and created them again by selecting the answer in the code, and using right click "Add answer placeholder", and the bounding boxes are the same I have committed (same values in the section `placeholders` in `task-info.yaml`). I am not sure if I am doing something wrong though. It is the first time I generate Edu content for PyCharm.
----------------------------------------------------------------
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] henryken commented on pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
henryken commented on pull request #11731:
URL: https://github.com/apache/beam/pull/11731#issuecomment-629801033
Thanks for the changes @iht. LGTM now.
@pabloem, can you help to merge and close this issue?
----------------------------------------------------------------
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] henryken commented on a change in pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
henryken commented on a change in pull request #11731:
URL: https://github.com/apache/beam/pull/11731#discussion_r426224090
##########
File path: learning/katas/python/Windowing/Adding Timestamp/ParDo/task-info.yaml
##########
@@ -22,10 +22,10 @@ files:
- name: task.py
visible: true
placeholders:
- - offset: 1211
- length: 163
+ - offset: 1243
Review comment:
We need to fix the answer placeholders.
##########
File path: learning/katas/python/Windowing/Adding Timestamp/ParDo/task.py
##########
@@ -16,6 +16,7 @@
import datetime
import time
Review comment:
Remove unused import
##########
File path: learning/katas/python/Windowing/Adding Timestamp/ParDo/task.py
##########
@@ -24,30 +25,30 @@
class Event:
- def __init__(self, id, event, date):
+ def __init__(self, id, event, timestamp):
self.id = id
self.event = event
- self.date = date
+ self.timestamp = timestamp
Review comment:
Need to change the `task.html` description
```html
Please assign each element a timestamp based on the the <code>Event.timestamp</code>
```
##########
File path: learning/katas/python/Windowing/Fixed Time Window/Fixed Time Window/task-info.yaml
##########
@@ -22,7 +22,7 @@ files:
- name: task.py
visible: true
placeholders:
- - offset: 2074
+ - offset: 2067
Review comment:
Need to fix the answer placeholder
----------------------------------------------------------------
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 merged pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #11731:
URL: https://github.com/apache/beam/pull/11731
----------------------------------------------------------------
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] iht commented on pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
iht commented on pull request #11731:
URL: https://github.com/apache/beam/pull/11731#issuecomment-629682959
R: @henryken
----------------------------------------------------------------
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 #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11731:
URL: https://github.com/apache/beam/pull/11731#issuecomment-630381512
thanks everyone!
----------------------------------------------------------------
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] iht commented on a change in pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
iht commented on a change in pull request #11731:
URL: https://github.com/apache/beam/pull/11731#discussion_r426235925
##########
File path: learning/katas/python/Windowing/Adding Timestamp/ParDo/task.py
##########
@@ -16,6 +16,7 @@
import datetime
import time
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
[GitHub] [beam] pabloem commented on pull request #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11731:
URL: https://github.com/apache/beam/pull/11731#issuecomment-630369162
making sure license tests pass
----------------------------------------------------------------
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 #11731: [BEAM-10018] Fix timestamps in two windowing Python katas
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11731:
URL: https://github.com/apache/beam/pull/11731#issuecomment-630363293
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