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

Running tests on minio #358

Merged
merged 1 commit into from Mar 10, 2020
Merged

Running tests on minio #358

merged 1 commit into from Mar 10, 2020

Conversation

fao89
Copy link
Member

@fao89 fao89 commented Feb 21, 2020

@fao89 fao89 force-pushed the 6155 branch 8 times, most recently from f77b79d to 0b953ef Compare February 27, 2020 18:17
@fao89 fao89 closed this Feb 27, 2020
@fao89 fao89 reopened this Feb 29, 2020
@fao89 fao89 force-pushed the 6155 branch 7 times, most recently from 9ca9765 to 109315c Compare February 29, 2020 14:12
@fao89 fao89 marked this pull request as ready for review February 29, 2020 15:16
@fao89 fao89 requested a review from a team February 29, 2020 15:16
@fao89
Copy link
Member Author

fao89 commented Feb 29, 2020

pulpcore tests are failing
https://travis-ci.org/pulp/pulp_file/jobs/656658172?utm_medium=notification&utm_source=github_status
for now, I'm running only pulp_file test

Comment on lines 96 to 98
# Hacking botocore:
sed -i "/gunicorn/a RUN sed -i \"s/hasattr(body, 'read')/getattr(body, '_size', None)/g\" \$(pip show botocore | grep -i location | awk '{ print \$2 }')/botocore/handlers.py" ./containers/images/pulp/Dockerfile.j2
cat ./containers/images/pulp/Dockerfile.j2
Copy link
Member Author

Choose a reason for hiding this comment

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

thank you @daviddavis
your tip led me to open this PR:
boto/botocore#1990

Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent I've subscribed to the PR and plan on following up next week if no one responds.

Can you add a link to your PR in the comments?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure!

@@ -89,8 +89,10 @@ export CMD_STDIN_PREFIX="sudo kubectl exec -i $PULP_API_POD --"
cat unittest_requirements.txt | $CMD_STDIN_PREFIX bash -c "cat > /tmp/test_requirements.txt"
$CMD_PREFIX pip3 install -r /tmp/test_requirements.txt

# Run unit tests.
$CMD_PREFIX bash -c "PULP_DATABASES__default__USER=postgres django-admin test --noinput /usr/local/lib/python${TRAVIS_PYTHON_VERSION}/site-packages/pulp_file/tests/unit/"
if [[ "$TEST" != 's3' ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we skipping the unit tests for s3?


def pytest_runtest_setup(item):
"""Adding delay."""
time.sleep(0.5)
Copy link
Contributor

Choose a reason for hiding this comment

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

This adds a 0.5 sleep before every test? Is that needed to throttle connections to minio?

If so, I wonder if we should make this a configuration setting so it doesn't impact non-minio test runs?

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot to remove that

@@ -173,7 +173,7 @@ def monitor_task(task_href):
completed = ["completed", "failed", "canceled"]
task = tasks.read(task_href)
while task.state not in completed:
sleep(2)
sleep(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just an optimization or is it minio related?

Copy link
Member Author

Choose a reason for hiding this comment

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

just an optimization, I think 2 seconds is too much

@daviddavis
Copy link
Contributor

Overall, great job. I think I like this solution better than the localstack one. What do you think?

@fao89
Copy link
Member Author

fao89 commented Feb 29, 2020

Overall, great job. I think I like this solution better than the localstack one. What do you think?

I agree minio is simpler

@@ -106,6 +113,11 @@ export PYTHONPATH=$TRAVIS_BUILD_DIR:$TRAVIS_BUILD_DIR/../pulpcore:${PYTHONPATH}

set -u

if [[ "$TEST" == 's3' ]]; then
pytest -v -r sx --color=yes --pyargs pulp_file.tests.functional || show_logs_and_return_non_zero
exit
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this to not run the pulpcore tests? If so, I have a PR to get them passing:

pulp/pulpcore#567

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it was because of that, I'm linking your PR as Required PR here

@fao89 fao89 force-pushed the 6155 branch 2 times, most recently from 4ee1ecb to 0ffe3db Compare March 1, 2020 21:49
@daviddavis
Copy link
Contributor

@mikedep333 @dkliban either of you want to review this before we merge and apply this to the plugin_template?

fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 4, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 4, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 5, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 5, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 5, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 5, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 5, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 6, 2020
aws_storage_bucket_name: '"pulp3"'
default_file_storage: '"storages.backends.s3boto3.S3Boto3Storage"'
media_root: ''
s3_use_sigv4: 'true'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, I forgot to run plugin_template after the last change, I'll do it for all PRs

fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 10, 2020
fao89 added a commit to fao89/pulpcore that referenced this pull request Mar 10, 2020
@daviddavis daviddavis merged commit 5b97616 into pulp:master Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants