Skip to content
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

Fix master builds #4869

Merged
merged 6 commits into from
Feb 19, 2020
Merged

Fix master builds #4869

merged 6 commits into from
Feb 19, 2020

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Feb 19, 2020

Fixes includes:

Update gitpython requirement (Thanks to @blag for tracking down the bug.)

Drop --no-site-packages from virtualenv calls

Make travis use the travis user instead of stanley, and add a restricted permissions workaround for the root user.

Update requests which had a conflicting dep on idna (primary said idna>=2.5,<2.9 but security extra wanted any idna>=2.0.0. idna 2.9 was released a couple of days ago, and requests 2.23.0 with fixed deps was released today.

gitpython requires gitdb2, but that has removed some things in newer
versions, and 2.11.1 had an open ended dep. So, update gitpython to a
newer version that has more specific version requirements for gitdb2.

This should fix these errors:
gitpython-developers/GitPython#983
https://travis-ci.org/StackStorm/st2/jobs/652088774#L9416
https://circleci.com/gh/StackStorm-Exchange/stackstorm-sumologic/155

Thanks to blag for tracking down the bug.
@pull-request-size pull-request-size bot added the size/XS PR that changes 0-9 lines. Quick fix/merge. label Feb 19, 2020
@cognifloyd
Copy link
Member Author

This won't fix all of the build errors on master. There are other issues as well, this is just the only one with a clear fix so far.

@cognifloyd
Copy link
Member Author

cognifloyd commented Feb 19, 2020

Other issues include:

  • something installing wrong idna version (.st2client-install-check in the Lint Checks build; can't find what changed here)
  • build pack virtualenv fails (in 2.7 Integation Tests build. Maybe due to permissions issue? Build has no details)
  • build timed out (py3 integration tests - probably just need to restart the build on travis)

Once we figure those issues out, they should probably go in separate PRs. Can we merge this fix even without resolving those other issues?

@cognifloyd cognifloyd requested a review from m4dcoder February 19, 2020 18:56
@m4dcoder
Copy link
Contributor

@cognifloyd If these issues are causing the errors in the CI above, then you have to fix them before we can merge.

@blag
Copy link
Contributor

blag commented Feb 19, 2020

NAK - investigating.

Nevermind, this PR should work just fine. I also removed the (long deprecated) --no-site-packages flag usage from the Makefile in my branch. Just wanted to make sure this covered all of the same bases.

There are more than one pack CI builds that are failing due to the same issue.

@blag blag added this to the 3.2.0 milestone Feb 19, 2020
@cognifloyd
Copy link
Member Author

cognifloyd commented Feb 19, 2020

I just cherry-picked the --no-site-packages fix from #4863.
Plus I'm trying to resolve the permissions issue in a nicer way than the xenial workaround to see if that fixes the 2.7 integration tests. that didn't affect it
Still stumped on the idna issue.

@pull-request-size pull-request-size bot added size/M PR that changes 30-99 lines. Good size to review. and removed size/XS PR that changes 0-9 lines. Quick fix/merge. labels Feb 19, 2020
@cognifloyd cognifloyd changed the title Update gitpython requirement Fix master builds Feb 19, 2020
@cognifloyd cognifloyd force-pushed the fix_gitpython_req branch 2 times, most recently from e4d3d69 to fa81ed5 Compare February 19, 2020 20:34
requests 2.22.0 had conflicting requirements on idna.
The primary requirement said idna>=2.5,<2.9 but the security extra
(which we use) required any idna>=2.0.0.

idna 2.9 was released 2 days ago
requests 2.23.0 was released today
@cognifloyd
Copy link
Member Author

It's GREEN! 💚

@blag blag merged commit 714179b into StackStorm:master Feb 19, 2020
@arm4b
Copy link
Member

arm4b commented Feb 20, 2020

It's arguable pip requirements fix needs that amount of changes. I'd prefer to not mix different changes in one PR. This keeps history clear and straightforward.

Additionally, per previous comment we're missing CHANGELOG record which is required for pip requirements update for st2 core.

Even though, many thanks for fixing master! ✔️

@cognifloyd
Copy link
Member Author

cognifloyd commented Feb 20, 2020

@blag asked me to get rid of the 777 permissions workaround affecting the entire home directory.
I'm not sure why precise started having permissions issues, but I needed to adjust something, so I moved some of the permissions changes from #4863 to this PR to resolve that, only I made sure to do something that would selectively change permissions only where needed.

Also, I offered to do separate updates for each of the issues and was told that nothing could be merged till it was all green. So, I fixed three issues in this PR to make it green.

So, no, this was not just a pip dep update.

Changelog PR coming soon. #4871

cognifloyd added a commit to cognifloyd/st2 that referenced this pull request Feb 20, 2020
cognifloyd added a commit to cognifloyd/st2 that referenced this pull request Feb 20, 2020
cognifloyd added a commit to cognifloyd/st2 that referenced this pull request Feb 21, 2020
cognifloyd added a commit to cognifloyd/st2 that referenced this pull request Feb 21, 2020
blag added a commit that referenced this pull request Feb 21, 2020
@cognifloyd cognifloyd deleted the fix_gitpython_req branch February 12, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M PR that changes 30-99 lines. Good size to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants