-
Notifications
You must be signed in to change notification settings - Fork 232
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
enable gset support #1665
enable gset support #1665
Conversation
This is failing quickcheck on my machine complaining about missing schema files:
On a previous run I got timeouts : rebase-nhs-riak.rdb.bet365.gsets.to.develop-2.2.timedout.txt And before that fail due to emfile - if this ever gets ported to rebar3 it might be worth adding a ulimit check (I try not to set it to 20000 by default). Not suggesting it's something we change now, just making a note for the future : |
Thanks for the review @bryanhuntesl, are these problems before or after merge to 2.2? They look like the test failures that were fixed in the 2.2.5 work that's already in 2.2. This is old, old code. Do you need me to rebase it? |
We really need to bring back thumbs bot |
If you could try rebasing - would be much appreciated. I'm giving it a fresh run right now. |
Ok , so on a clean checkout of develop-2.2 I get a bunch of timeouts (6 cancelled tests) and these warnings from above (see attached file). wednesday.develop-2.2-attempt.txt I'll try the rebase + test and separately the merge and test again now. |
Hmm, all those problems in the tests are fixed afaict in develop-2.2. It's a shame we keep doing this. |
WRT the timeouts…are you running in some kind of constrained environment? It's not something I've seen running these tests 100s (maybe 1000s) of times since I merged the test fix branches into 2.2 |
Nope, this is on a fairly new Mac Pro, latest updates.
…On Thu, 22 Mar 2018, 10:26 Russell Brown, ***@***.***> wrote:
WRT the timeouts…are you running in some kind of constrained environment?
It's not something I've seen running these tests 100s (maybe 1000s) of
times since I merged the test fix branches into 2.2
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1665 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/Aego9rHkyvrj1vepi_YOKvF-CmJzD-Pgks5tg3xPgaJpZM4SYTxN>
.
|
the thing is, I don't know how to approach this. Do those same tests behave the same way without this change? If so…unrelated, right? We know we're starting from an imperfect point, so to question that same imperfection at every PR…we know that there are issues with the underlying code. But we can't fix all tech debt before making any forward progress. Do you think that the failures are related to the PR? I'm still waiting for them to finish running…the nhs-riak fork passes on 2.2 every time and has this change merged. When they finish I'll run basho-2.2, then this branch (I'd expect this branch to fail thanks to it's original 2.2 base being broken) and then the merge of the two. If it works for me, and continues to not work for you, we're kind of at an impasse, eh? |
|
Hmm, so there is defo a commit missing from basho-2.2 that is in nhs-2.2…I have to go hunting |
excellent ! I'd just started another run of just develop-2.2 in case it was intermittent. |
This seems to only be an issue when you run |
well let you know OOPS,WRONG BUTTON |
Soooo…it looks like OpenRiak@6bcfb1f was missing from basho-2.2, so I added it (#1666) I ran tests with that merged to 2.2, and 2.2 passed for me. Hopefully that waterfall of joy happens for others too |
Tests pass - with that #1666 merged. Beautiful. +1 ship it. |
awesome, thanks for you patience/perseverance! |
Adding GSet support to 2.2. It was merged into basho/riak_kv develop, but not a 2.2 branch afaict.
Tests at basho/riak_test#1302
Needs
basho/riak-erlang-client#373
basho/riak-erlang-http-client#70
basho/riak_pb#229
basho/riak_test#1302