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

Attempt at heartbeat pre-merging #33

Merged
merged 8 commits into from May 25, 2018

Conversation

Projects
None yet
2 participants
@johan-bjareholt
Copy link
Member

johan-bjareholt commented May 12, 2018

No description provided.

@johan-bjareholt

This comment has been minimized.

Copy link
Member

johan-bjareholt commented May 12, 2018

Squash these commits when done

johan-bjareholt added some commits May 13, 2018

endpoint = "buckets/{}/heartbeat?pulsetime={}".format(bucket_id, pulsetime)
data = event.to_json_dict()

This comment has been minimized.

@ErikBjare

ErikBjare May 14, 2018

Member

I don't think premerging of heartbeats should be done at all if queued=False, would make the code clearer as well I think.

This comment has been minimized.

@johan-bjareholt

johan-bjareholt May 14, 2018

Member

True.

Something unrelated to this PR though, queue should probably be on by default. In most cases the watcher should be dumb and only send data and not really care about the response.

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented May 14, 2018

Just saw your message in Messenger saying "this is completely broken". Please elaborate.

@johan-bjareholt

This comment has been minimized.

Copy link
Member

johan-bjareholt commented May 14, 2018

@ErikBjare Since the "pre-merge attempt 2" commit it is not completely broken anymore!

johan-bjareholt added some commits May 14, 2018

Response is no longer optional in _get/_post/_delete
There's either a response or there's a raised exception

@johan-bjareholt johan-bjareholt force-pushed the dev/heartbeat-pre-merge branch from 1d52137 to b4187e7 May 14, 2018

@johan-bjareholt johan-bjareholt referenced this pull request May 15, 2018

Merged

Added CLI script #30

2 of 5 tasks complete
@johan-bjareholt

This comment has been minimized.

Copy link
Member

johan-bjareholt commented May 24, 2018

Please review, works well

@ErikBjare
Copy link
Member

ErikBjare left a comment

Looks good to me!

@johan-bjareholt johan-bjareholt merged commit dca4d4e into master May 25, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@wafflebot wafflebot bot removed the review label May 25, 2018

@johan-bjareholt johan-bjareholt deleted the dev/heartbeat-pre-merge branch May 25, 2018

@ErikBjare ErikBjare referenced this pull request May 29, 2018

Closed

Performance improvements #98

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment