Conversation
|
@blag I'm not sure I understand the PR here. The title says backoff some of the dependency changes but the commits here are upgrading the dependencies. Can you provide some more context? |
d1d095e to
4a0e8a1
Compare
|
Sure, so the originating context here is that a few of the weekly pack CI runs have been failing and I took it upon myself to fix them if I could. One of them was the stackstorm-git pack, which failed with this error:
@Kami had updated the dependency version of amqp to 2.5.0 in #4767, which also updated kombu to v4.6.4, which in turn requires So #4767 broke anything that tried to install the dependencies in So I upgraded the required amqp version to 2.5.1, and then needed to run At some point I ran across the error where pip had installed PyYAML v5.1.2, but the pack constants still expected The requests library upgrade was good, except that prance v0.6.1 required The next failure I came upon was where I wasn't done with dealing with that section of the Makefile though, because immediately after that it uninstalls pytz, python-dateutil, and orquesta, and then runs I also added instructions on how to update the I've updated the title of this PR to more accurately reflect what it's attempting to achieve. |
|
It's not just the weekly pack CI runs that are broken, it's also pack PR CI jobs that are broken: |
|
👍 The good, - it closes #4770 I'm also unsure why do we need Mac OS X build and what consequences it brings. It was discussed several times before that we don't support any OSX-related builds and make sure tries to do that don't affect any st2 code. |
|
I would rather have a roadmap bullet point slip than break all pack CIs. ST2 still doesn't build on OS X (and likely never will), but at least we can run |
|
From the rabbit hole investigation you described in detail #4774 (comment), how the resulting code would differ in this PR without OS X requirements, eg. if making requirements on OS we officially support? |
|
The only pieces that are Mac OS X support are: And even with this change, we still only officially support building on Linux. The build still ultimately fails on OS X, since the Python inotify package doesn't successfully install on anything but Linux. I did make sure that |
a50f8af to
294ecf0
Compare
arm4b
left a comment
There was a problem hiding this comment.
Please include 1 bugfix Changelog record for this PR instead of 3, make it brief and clear.
|
@blag I appreciate the effort you put into getting all of this to work. I'm OK w/ the fixes here. The reason with my delayed response here is because the changes are a mishmash of dependency fixes. I have to reread your explanation several times. I came to the conclusion here that this fixes a number of dependency issues which plagues exchange CI. It'll be unproductive to unnecessary block this. Maybe we do need a better strategy to manage dependency. But until we identify a solution, I think it will be helpful in the future for all of us to be very succinct about grouping changes and keeping in scope. For example, OSX is not a supported distro. For efficiency sake, the recommended platform for developers should be on Linux. I see little value in maintaining whatever compatibility or functionality on OSX. |
294ecf0 to
62c67ba
Compare
|
Resolved all requested changes. |
This PR backs off on some of the updates to dependencies, and makes
make requirementsrun successfully, on both Linux and Mac OS X (I tested both). This should also hopefully fix the weekly pack Circle CI runs.I also tweak the requirements header to include directions on how to actually update dependencies, since it's definitely not a straightforward process.
On top of that, I also make sure the make targets actually fail, so (hopefully) future updates that break things won't pass CI.
Closes #4770 and #4771.
Reverts parts of #4689 and #4767.