Upgrade dependencies to work with ruby 2.4, tweaked rspecs #21
Conversation
kigster
commented
Jun 9, 2017
kigster
commented
- reduced sleep time during specs; seems to still pass
- made rspecs run in random order
- that uncovered a couple of unstable specs which I think I fixed
- turned off logging in specs unless ENV['DEBUG'] is set
- remove version binding from bundler, rake
- upgraded avro and redis gems.
- reduced sleep time during specs; seems to still pass - made rspecs run in random order - that uncovered a couple of unstable specs which I think I fixed - turned off logging in specs unless ENV['DEBUG'] is set - remove version binding from bundler, rake - upgraded avro and redis gems.
|
My theory is that the specs are polluting thread space or something, basically the order in which you run tests matters, which is not good :( It's hard for me to tell whether tests are flopping because of my tiny changes, or if they'd flop even before. I guess I could submit a PR that ONLY has a random spec order defined, and no other changes, but right now I am not sure that this library is stable enough for wide adoption. I'll keep digging though. |
|
@kigster Ruby threads can be hard, any improvement to the test suite is super welcome! In terms of stability we are processing 10s of millions of events a day on multiple shards with multiple consumers using this library. I don't know what your use looks like, but it has proven stable for our needs. |
|
That's good to know, let me keep looking at the code and see. And as far as threads are concerned, what if the library used Celluloid instead of threads for concurrency? I noticed that you are also using AWS SDK v1, which is obsolete at this point. Version 2 of the SDK seems to work out of the box, based on my testing following examples on the SDK documentation. So I sort of see the evolution of this library, that uses Actors for concurrency, Config gem for configuration, and AWS SDK v2 for Kinesis, while the rest can probably be the same. If this sounds like a good idea, I am happy to evolve this PR. But if not, I may just create a new project but perhaps thank this gem for the inspiration :) . Thoughts? |
|
@kigster To be honest I think we are unlikely to want to invest any significant amount of time QAing major updates to this library on our current stack and with our current workload, especially with how stable and hassle free this has been for us so far. If you want to make significant changes a separate library may be a better option. I have also started an Elixir implementation of this library that may be of interest to you, particularly if you look at the celluloid stuff: ello/ello-event-stream#1 I do hate to discourage you from contributing and your issues/PRs are very much appreciated and I hope the library helps you! Unfortunately this is just low priority for Ello as a company, and not something I am personally passionate enough about to help push forward in our production environment. |
|
That's absolutely understandable, and is a entirely respectable choice, which I appreciate knowing before spending hours on it :) Thanks for sharing you code anyway! |
