Skip to content
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

Fix watch (-w) #196

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix watch (-w) #196

wants to merge 4 commits into from

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented May 28, 2018

Delete nextToken from kwargs, otherwise this will cause the response
to be stale / the same always.

This should get improved probably to update kwargs['startTime']
probably, but it is not clear to me if we could use the max or min
from the received events here. Maybe with some offset being applied for
safety (self.parse_datetime("5m ago"))?

Fixes #195.

@coveralls
Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage decreased (-1.3%) to 90.753% when pulling 883a473 on blueyed:fix-watch into a6ad3d6 on jorgebastida:master.

@blueyed
Copy link
Contributor Author

blueyed commented May 28, 2018

Only lint is failing (fixed in #194).

Should I look into adding a test for this?

blueyed added a commit to blueyed/awslogs that referenced this pull request May 28, 2018
All tests were passing with some `assert 0` to see which tests cover jorgebastida#196,
which should not happen.
@blueyed
Copy link
Contributor Author

blueyed commented Dec 2, 2018

Rebased.

@blueyed
Copy link
Contributor Author

blueyed commented Dec 2, 2018

See #142 (comment) for what seems to be needed here, too.
Should probably take out the unrelated things here?!

@blueyed
Copy link
Contributor Author

blueyed commented Dec 2, 2018

Actually this handles startTime already, and should be fine I guess?!

Delete `nextToken` from kwargs, otherwise this will cause the response
to be stale / the same always.

This should get improved probably to update `kwargs['startTime']`
probably, but it is not clear to me if we could use the `max` or `min`
from the received events here.  Maybe with some offset being applied for
safety (`self.parse_datetime("5m ago")`)?
Eventually this should get refactored, but it seems better to just
silence mccabe for now.

It also uses the default max-complexity for the rest now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants