Skip to content

Conversation

@MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli commented Dec 19, 2022

@MarcoGorelli MarcoGorelli marked this pull request as ready for review December 19, 2022 12:31
@MarcoGorelli MarcoGorelli added the Dependencies Required and optional dependencies label Dec 19, 2022
@MarcoGorelli MarcoGorelli changed the title CI try adding minimal reqs file CI: add minimal requirements file Dec 19, 2022
@MarcoGorelli MarcoGorelli added the CI Continuous Integration label Dec 19, 2022
@WillAyd
Copy link
Member

WillAyd commented Dec 20, 2022

Love the idea - needed something similar for #50046

Wondering if we shouldn't make this actually a minimal requirements file though, which would exclude pytest-cov and pytest-xdist (I think only CI needs those). Might be nice to have a true minimal file and compose the CI required-elements on top of that

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@WillAyd
Copy link
Member

WillAyd commented Jan 3, 2023

Any other thoughts on this? @mroeschke @lithomas1 @fangchenli

python -m pip install git+https://github.com/nedbat/coveragepy.git
python -m pip install versioneer[toml]
python -m pip install python-dateutil pytz cython hypothesis==6.52.1 pytest>=6.2.5 pytest-xdist pytest-cov pytest-asyncio>=0.17
python -m pip install -r ci/deps/requirements-minimal.txt pytest-cov pytest-xdist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, are you sure pytest-xdist is not required? I remember having to install it once for some test(it was using one of the pytest-xdist fixtures).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the S3 tests use the worker_id pytest-xdist fixture. All of them should be marked with single_cpu so I guess in this job those should be skipped. Might make sense to include pytest-xdist just in case anything changes

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Feb 4, 2023
@WillAyd
Copy link
Member

WillAyd commented Feb 4, 2023

We've got 2 approvals on this. Rather then let it go stale let's merge in on next conflict fixup, unless someone objects

@MarcoGorelli MarcoGorelli added this to the 2.0 milestone Feb 8, 2023
@MarcoGorelli MarcoGorelli removed the Stale label Feb 8, 2023
@MarcoGorelli
Copy link
Member Author

this workflow isn't actually running at the moment, so marking as draft until it's re-enabled #50696 (comment)

@MarcoGorelli MarcoGorelli marked this pull request as draft March 2, 2023 10:06
@mroeschke mroeschke modified the milestones: 2.0, 2.1 Mar 16, 2023
@MarcoGorelli
Copy link
Member Author

closing to clear the queue, might pick it up again in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration Dependencies Required and optional dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: add minimal requirements file

4 participants