-
-
Notifications
You must be signed in to change notification settings - Fork 782
Revert mongoengine dependency upgrade #3402
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
Conversation
|
Good idea. Will keep an eye on this. Curious how you knew to look for this - how long has this been an issue? I was going to start looking through history last night but that was at about 1AM so I bailed EDIT: nevermind, old build node thinking. Looking at history of new build node, clear that this wasn't working since its inception. So could have been broken before, but definitely broken as of #3372 |
|
Okay, u16 was passing with your reverts, so I set mongoengine back to 0.13 in 31c6065 |
|
Just changing mongoengine back to 0.13 re-broke things. So it seems pretty clear that this upgrade is the culprit |
Signed-off-by: Matt Oswalt <oswaltm@brocade.com>
Signed-off-by: Matt Oswalt <oswaltm@brocade.com>
|
Okay I reverted the reverts except for the one for mongoengine. Will watch this make sure this passes, then we should consider merging this for now so that tests for other PRs can go through. Someone knowledgeable about the database layer (not me) should review http://docs.mongoengine.org/changelog.html#changes-in-0-12-0 and see if anything impacts us; it seems obvious that it's something, but I am not sure. |
|
👍 I did check the changelog before bumping, but nothing stood out and everything also passed locally and on Circle CI. Having said that, it does look like some change is negatively impacting us so I will need to dig deeper in the mongoengine code. |
|
As discussed with @Mierdin we 🚒 the 🔥 first and merge the fix to unblock the CI. According to the commit + CI And @Kami feel free to dig in deeper into the |
| # Since states are being processed asynchronously, wait for the | ||
| # liveactions to go into scheduled states. | ||
| for i in range(0, 100): | ||
| eventlet.sleep(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was actually one because concurrency tests were failing, but if they are not failing anymore, it's fine.
That failure was probably related to Circle CI since it was failing on master as well, before any other stuff was merged.
Reverts #3372
For now just a PR to experiment if
pippackages might be related to re-appeared Ubuntu16 regression (even with timeouts) StackStorm/st2-packages#445.