Skip to content
This repository has been archived by the owner on Mar 11, 2022. It is now read-only.

fixes #27 – remove entity context for a cancelled stream. #28

Merged
merged 1 commit into from
Jun 15, 2020

Conversation

marcellanz
Copy link
Contributor

@marcellanz marcellanz commented Jun 13, 2020

Fixes #27 – "Entity throws error after 30 seconds of inactivity".

Please find a detailed analysis and script to reproduce the issue on the issue itself.

  • the fix is valid for v0.1.0 and we'll release v0.1.1. The next milestone release will handle the entity context differently and this fix will not apply for the work currently done on the crdt branch which is the basis for milestone v0.2.0.
  • Entity throws error after 30 seconds of inactivity #27 lists a shell script where the fix can be validated.
  • a TCK image for v0.1.1 is already pushed to docker hub for verification.

@codecov-commenter
Copy link

Codecov Report

Merging #28 into master will decrease coverage by 0.23%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
- Coverage   45.62%   45.39%   -0.24%     
==========================================
  Files           9        9              
  Lines         572      575       +3     
==========================================
  Hits          261      261              
- Misses        266      269       +3     
  Partials       45       45              
Impacted Files Coverage Δ
cloudstate/cloudstate.go 30.17% <ø> (ø)
cloudstate/eventsourced.go 44.55% <0.00%> (-0.68%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bd8eb4...177f7b3. Read the comment docs.

Copy link
Member

@pvlugter pvlugter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Sounds good to have a quick fix now and the proper handling added to 0.2. Good to see the explanation and test in the issue.

@ralphlaude
Copy link

@marcellanz i have no idea on Go, but i see the context is deleted. Cool with the quick fix. It looks good.

@marcellanz
Copy link
Contributor Author

i have no idea on Go, but i see the context is deleted. Cool with the quick fix. It looks good.

For sure a quick fix @ralphlaude for a dirty bug. But as I've mentioned, the rewrite done for the coming release with the CRDT support will have removed that global state, and also rewritten a few other not so clean things.

@marcellanz marcellanz merged commit d87dac5 into master Jun 15, 2020
@marcellanz
Copy link
Contributor Author

thanks for the review @pvlugter & @ralphlaude

@marcellanz marcellanz deleted the fix/go27_stream_cancellation branch June 26, 2020 14:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entity throws error after 30 seconds of inactivity
4 participants