-
Notifications
You must be signed in to change notification settings - Fork 150
Reimplement #201 #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reimplement #201 #231
Conversation
Some effect as python-streamz#201, but should now pass tests
|
i.e., add "from_end" parameter to from_textfiles, which only makes events out of new data. |
Codecov Report
@@ Coverage Diff @@
## master #231 +/- ##
==========================================
+ Coverage 93.38% 93.46% +0.08%
==========================================
Files 13 13
Lines 1481 1485 +4
==========================================
+ Hits 1383 1388 +5
+ Misses 98 97 -1
Continue to review full report at Codecov.
|
streamz/tests/test_core.py
Outdated
| out = source.sink_to_list() | ||
| source.start() | ||
| assert out == [] | ||
| yield gen.sleep(0.01) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little nit-picky, but why this sleep?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seek() to end doesn't happen until the coroutine runs for the first time, so if you were to write immediately here, you would always seek past the new data. This yield makes sure the coroutine runs once. Would a yield without the sleep have the same effect, or is there another way to say "run coroutines now"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you just want one cycle to go through, then try yield gen.moment.
If you might need a few cycles, then you should probably have an await_for
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try moment tomorrow; await_for is tricky when you don't expect the output to change. Perhaps it would be reasonable to await on at attribute of the source.
|
btw: when I run flake locally, I get loads more issues than show up in travis; how is this configured? |
|
I'm not sure. I would check the procedure in the .travis.yml file. Maybe
a different version of flake8?
…On Thu, Mar 21, 2019 at 7:09 AM Martin Durant ***@***.***> wrote:
btw: when I run flake locally, I get loads more issues than show up in
travis; how is this configured?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#231 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AASszH25-SD2HZ1VMNr93XDwf58T0nN8ks5vY5KrgaJpZM4cATY6>
.
|
|
I believe this is ready (and should close #201) |
|
Fixes #199 |
Some effect as #201, but should now pass tests