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.

Sign up for GitHub

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

Jump to bottom

Use pants managed venv in Makefile #6130

Closed
wants to merge 92 commits into from
Closed

Use pants managed venv in Makefile #6130

wants to merge 92 commits into from

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Feb 10, 2024 •
edited
Loading

I don't intend to merge this. I want to use this to find any issues with the requirements pants is using and limit them appropriately. So, the changes to the Makefile are quite hacky.

@cognifloyd cognifloyd added this to the pants milestone Feb 10, 2024
@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. and removed size/L PR that changes 100-499 lines. Requires some effort to review. labels Feb 10, 2024
@cognifloyd cognifloyd force-pushed the pants-export-venv branch 5 times, most recently from 7190169 to 8baa519 Compare February 10, 2024 23:25
@cognifloyd cognifloyd mentioned this pull request Feb 16, 2024
Merged
@cognifloyd cognifloyd added the no changelog No Changelog.rst needed for this PR label Feb 16, 2024
@cognifloyd
Copy link
Member Author

cognifloyd commented Feb 16, 2024 •
edited
Loading

The pants-managed venv does not have PyOpenSSL.

I don't know if that's related to the timeouts in the integration tests or not.

Nope. That's irrelevant because we're already pinned to requests==2.25.1, so we're only getting pyopenssl because of our pin now. If we dropped that it would probably go away. We can also drop the [security] extra since it does nothing.

@cognifloyd
rework self-check ci to work with pants venv
50d61b9
@cognifloyd
refactor ci
8bd1f50
@cognifloyd
Try w/o the PyOpenSSL req hack
f36285a 
GHA isn't failing with the RabbitMQ errors anymore.
Hopefully the hack isn't actually needed.

Lockfile diff: lockfiles/st2.lock [st2]

==                     Removed dependencies                     ==

  pyopenssl                      24.1.0
@cognifloyd
it worked. drop commented bits.
d35a99f
@cognifloyd
Drop amqp constraint
9720e5c 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  amqp                           5.0.6        -->   5.2.0
  vine                           5.0.0        -->   5.1.0
@cognifloyd
drop bcrypt constraint
1bd30bc 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  bcrypt                         3.2.2        -->   4.1.2
@cognifloyd
drop greenlet constraint
39a3299 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  greenlet                       1.1.3.post0   -->   3.0.3
@cognifloyd
drop tenacity constraint
2443c82 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  tenacity                       6.3.1        -->   8.2.3
@cognifloyd
drop oslo.utils constraint
bd95f5c 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  oslo-utils                     4.13.0       -->   7.1.0
@cognifloyd
drop cffi constraint
3317c30 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  cffi                           1.14.6       -->   1.16.0
@cognifloyd
drop kombu constraint
f3c2401 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  kombu                          5.0.2        -->   5.3.6

==                      Added dependencies                      ==

  typing-extensions              4.11.0
@cognifloyd
drop stevedore constraint
2e90551 
Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  stevedore                      2.0.1        -->   5.2.0
cognifloyd added a commit that referenced this pull request Apr 10, 2024
@cognifloyd
Pants: Minimize/Update 3rd party requirements constraints
6024bdc 
Using the pants venv, the tests all pass with these requirements.
I hacked the Makefile to test that in CI in #6130.
This extracts just the requirements bits from that PR.

Lockfile diff: lockfiles/st2.lock [st2]

==                    Upgraded dependencies                     ==

  amqp                           5.0.6        -->   5.2.0
  bcrypt                         3.2.0        -->   4.1.2
  cffi                           1.14.6       -->   1.16.0
  eventlet                       0.30.3       -->   0.36.1
  filelock                       3.13.3       -->   3.13.4
  kombu                          5.2.2        -->   5.3.6
  oslo-utils                     4.13.0       -->   7.1.0
  stevedore                      2.0.1        -->   5.2.0
  tenacity                       6.3.1        -->   8.2.3
  vine                           5.0.0        -->   5.1.0

==                      Added dependencies                      ==

  typing-extensions              4.11.0
@cognifloyd cognifloyd mentioned this pull request Apr 10, 2024
Merged
cognifloyd

This comment was marked as resolved.

Sign in to view

@pull-request-size pull-request-size bot added size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. and removed size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. labels Apr 15, 2024
@cognifloyd
Add pants+pex hermetic_scripts workaround to st2-run-pack-tests
95c8b57 
By default, pants+pex adds `-sE` to the virtualenv/bin scripts which prevents
PYTHONPATH and other PYTHON* vars from affecting the program.

st2-run-pack-tests uses PYTHONPATH, however, so bypass the script if it is hermetic.
@cognifloyd
drop hermetic_scripts workaround from Makefile
5a66bce 
to make sure this new method works for running nosetests.
This was referenced Apr 15, 2024
Merged
Merged
Merged
@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. labels Apr 23, 2024
@cognifloyd
Copy link
Member Author

Closing. The requirements changes have already been merged and the remaining changes were implemented in a cleaner way in other PRs that have been merged and will be merged. The branch will still be available if someone needs to consult changes made to the Makefile or similar.

@cognifloyd cognifloyd closed this Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
external dependency infrastructure: ci/cd no changelog No Changelog.rst needed for this PR pantsbuild size/L PR that changes 100-499 lines. Requires some effort to review. tests
Projects
None yet
Milestone
pants
Development

Successfully merging this pull request may close these issues.
2 participants
@cognifloyd @winem