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

Fixing breaking change to Concurrent::Future in Crystal 0.35 #2

Merged
merged 4 commits into from
Sep 8, 2020

Conversation

konung
Copy link
Contributor

@konung konung commented Sep 7, 2020

  1. Crystal team has move Concurrent::Future to community-future shard, and remove functionality from standard library in Crystal version 0.35
  2. Discussion is here: Drop Concurrent::Future and top-level methods delay, future, lazy crystal-lang/crystal#9093
  3. This update lucky-cluster to be compatible with the change

On a side note, Amber::Cluster actually spawns separate processes via Process.fork - worth looking into.

@konung
Copy link
Contributor Author

konung commented Sep 7, 2020

There are no specs to run so it fails, but I added to luckyframework-website on my mac just to test, and it works as expected.

@konung
Copy link
Contributor Author

konung commented Sep 7, 2020

image

Screenshot of luckyframework website runing wiht MAX_THREADS=16

There is no measurable difference on Mac. But per your comment in Readme, it sounds like I need to try this on Linux

Copy link
Owner

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for the update. This all looks good to me. Yeah, I saw how Amber was handling this, but this method was based off spider-gazelle. Might be worth checking in to again later. Thanks for the heads up.

@jwoertink jwoertink merged commit e1b03a3 into jwoertink:master Sep 8, 2020
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