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

CI: Mark s3 tests parallel safe #35895

Merged
merged 3 commits into from
Aug 26, 2020

Conversation

TomAugspurger
Copy link
Contributor

Closes #35856

I think we need to update the pytest pattern though, so this should
fail.

Closes pandas-dev#35856

I think we need to update the pytest pattern though, so this should
fail.
@jbrockmendel
Copy link
Member

so this should fail.

Does this mean that this PR should not be merged despite being green?

@TomAugspurger
Copy link
Contributor Author

I'm going to rerun it a few times. I suspect we'll get a failure.

@TomAugspurger TomAugspurger changed the title CI: Mark s3 tests as single CI: Mark s3 tests parallel safe Aug 25, 2020
@TomAugspurger
Copy link
Contributor Author

Updated to just make these fixtures parallel safe. At the start of each session, each worker starts up a moto server on localhost:555{worker_id}. Then we just need to plumb that through everywhere.

I think Martin mentioned possible memory issues on windows workers with this approach, but that may have been starting the moto server at the test level. Hopefully doing things at the session level avoids that.

@TomAugspurger
Copy link
Contributor Author

Rerunning azure at https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=40904&view=results. But we should be good.

@TomAugspurger TomAugspurger added CI Continuous Integration Unreliable Test Unit tests that occasionally fail labels Aug 25, 2020
@TomAugspurger
Copy link
Contributor Author

#35655 wasn't backported, so this shouldn't need a backport either.

@jorisvandenbossche jorisvandenbossche added this to the 1.2 milestone Aug 25, 2020
@TomAugspurger
Copy link
Contributor Author

I've seen a couple failures on re-runs, but nothing to do with s3 / moto. So this should be good to go.

@TomAugspurger TomAugspurger merged commit 9f3e429 into pandas-dev:master Aug 26, 2020
@TomAugspurger TomAugspurger deleted the 35856-single branch August 26, 2020 02:21
@@ -34,12 +34,13 @@ def feather_file(datapath):


@pytest.fixture
def s3so():
return dict(client_kwargs={"endpoint_url": "http://127.0.0.1:5555/"})
def s3so(worker_id):
Copy link
Member

Choose a reason for hiding this comment

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

@TomAugspurger where is this defined? I'm getting test failures locally, guessing i need to update some dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

worker_id is from pytest-xdist.

Copy link
Member

Choose a reason for hiding this comment

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

test_s3_roundtrip_for_dir is failing in the CI and im trying to reproduce locally but it is getting pytest.skipped and it isn't clear why. could worker_id be involved?

@martindurant
Copy link
Contributor

I think Martin mentioned possible memory issues on windows workers

Yes, this was for the case of restarting the process for every test (i.e., standard pytest scope) - I believe moto may be less than optimal at cleaning up on exit, at least on Windows.

@simonjayhawkins simonjayhawkins mentioned this pull request Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Unreliable Test Unit tests that occasionally fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: specified bucket does not exist in TestParquetPyArrow.test_s3_roundtrip_explicit_fs
4 participants