-
-
Notifications
You must be signed in to change notification settings - Fork 782
Upgrade various dependencies (v2.4.0) #3538
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
| mongoengine==0.11.0 | ||
| gitpython==2.1.5 | ||
| jsonschema==2.6.0 | ||
| mongoengine==0.13.0 |
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 is the same change which we reverted before: #3402
It caused the regression for Xenial when 'st2ctl register' failed to register rules on a fresh DB.
If you could investigate and fix the root cause of StackStorm/st2-packages#445 without adding timeouts, - I'm good with that.
Otherwise let's please avoid the same mongoengine update we reverted before.
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.
Yeah, plan is to merge it once more testing is done and that issue is resolved.
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.
I believe this change should do it - 60a9dde.
As mentioned in that ticket and discussed on Slack, it seems like there is some weird race and internal trigger types are not registered yet when the script runs.
Registering internal triggers is an idempotent operation we do at every service setup / init phase so doing it again here should have no negative consequences (perhaps just a small performance overhead, but should be fine).
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.
Also, as we just discussed offline, you are correct, this script shouldn't rely on other services to start first and with this change this is now indeed the case.
In the past we didn't do that, because registering internal trigger types talks to the database and we thought register-content will also be able to run on other servers without st2 and talk to st2 over API, but that's not the case and other functionality of the script also requires to be run from the server where st2 components are running.
And if we ever move it to the API, that's fine as well because then script won't require DB access anymore.
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.
We believe #3542 should resolve the actual underlying race issue so we should be good, but we will still of course do more testing.
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.
👍
|
Sadly we still can't upgrade to the latest version of eventlet. v0.20.0 removes |
st2-register-content script. This should resolve the weird race issue @Arma spotted on Ubuntu 16.04.
arm4b
left a comment
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.
With the #3542 fix mongoengine upgrade looks good to me 👍
Cant say about other dependencies.
|
+1 interested in this based on a conversation in slack about HA and coordination backends. It appears there is a bug in the current version of tooz that prevents the postgres driver from connecting properly. |
|
Upgrading eventlet is quite challenging because of removal of |
|
@nmaludy also updated tooz to latest version, those changes should be included in v2.4.0 (should be out some time in the next couple of months). If it's a small bug fix, we can potentially also pick that change in v2.3.2. |
|
|
||
| import six | ||
| import requests | ||
| import urllib3 |
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.
Just some house keeping and another attempt after #3189 didn't make it.
It updates the following dependencies to the latest stable version.
General dependencies
eventletThe plan is to include those changes in v2.4.0 after some more testing is done.
I did go over all the changelog entries and now I'm looking into unit tests to make sure that all of them pass (there are some changes needed, but mostly because some libraries doesn't expose good enough APIs so we need to write some less than ideal and robust code to make things happen - e.g. how we throw more user-friendly exceptions for some jsonschema validation failures).
TODO