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

Fixes #37929 - support download policies for file #11184

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

sbernhard
Copy link
Member

@sbernhard sbernhard commented Oct 18, 2024

Support immediate and on-demand for file repos.

@sbernhard sbernhard marked this pull request as draft October 18, 2024 15:52
@sbernhard sbernhard changed the title Fixes #37929 - support policies for python/file Fixes #37929 - support download policies for file Oct 20, 2024
@sbernhard sbernhard force-pushed the fix_37929 branch 2 times, most recently from 9f0df87 to fde6027 Compare October 22, 2024 10:43
@sbernhard sbernhard marked this pull request as ready for review October 23, 2024 07:37
@ianballou
Copy link
Member

Did all of the VCR tests here need to be rerecorded? We only rerecord all at once when doing Pulp upgrades, unless a feature seems to actually touch >~70% of the VCR tests.

As for why some are failing still, there could be a couple reasons. The Pulp bindings you used to record the VCRs could be different from what's used in CI here (bundle update will fix it). Or, there could be a random element in the test that is being passed to the Pulp API bindings. Random values being sent to the API will always cause VCR errors because they'll no longer match the recorded VCR.

@sbernhard sbernhard force-pushed the fix_37929 branch 5 times, most recently from 0f9c380 to 225f451 Compare October 26, 2024 08:23
@sbernhard
Copy link
Member Author

Did all of the VCR tests here need to be rerecorded? We only rerecord all at once when doing Pulp upgrades, unless a feature seems to actually touch >~70% of the VCR tests.

As for why some are failing still, there could be a couple reasons. The Pulp bindings you used to record the VCRs could be different from what's used in CI here (bundle update will fix it). Or, there could be a random element in the test that is being passed to the Pulp API bindings. Random values being sent to the API will always cause VCR errors because they'll no longer match the recorded VCR.

Thanks for the help. Updated only the necessary vcrs. Actually one test failing - and no clue why 😤

@ianballou
Copy link
Member

Error: Failure: test_chunk_upload

You likely need to set 'ORPHAN_PROTECTION_TIME = 0' in pulp's settings.py.

Does test/services/katello/pulp3/content_test.rb pass locally for you? I wonder if it was rerecorded with a Pulp state that caused it to fail.

@sbernhard
Copy link
Member Author

Error: Failure: test_chunk_upload

You likely need to set 'ORPHAN_PROTECTION_TIME = 0' in pulp's settings.py.

Does test/services/katello/pulp3/content_test.rb pass locally for you? I wonder if it was rerecorded with a Pulp state that caused it to fail.

currently I run into:

Error:
Katello::Service::Pulp3::ContentTest#test_chunk_upload:
PulpFileClient::ApiError: Error message: the server returns an error
HTTP status code: 503
Response headers: {"date"=>"Mon, 04 Nov 2024 14:25:38 GMT", "server"=>"Apache", "content-length"=>"299", "connection"=>"close", "content-type"=>"text/html; charset=iso-8859-1"}
Response body:

<title>503 Service Unavailable</title>

Service Unavailable

The server is temporarily unable to service your request due to maintenance downtime or capacity problems. Please try again later.

/home/vagrant/katello/app/services/katello/pulp3/repository.rb:195:in `list'
/home/vagrant/katello/test/support/pulp3_support.rb:26:in `ensure_creatable'
/home/vagrant/katello/test/support/pulp3_support.rb:74:in `create_repo'
/home/vagrant/katello/test/services/katello/pulp3/content_test.rb:20:in `setup'
/home/vagrant/katello/test/support/vcr.rb:24:in `block in run'
/home/vagrant/katello/test/support/vcr.rb:23:in `run'

@sbernhard
Copy link
Member Author

Looks like, the tests are now green @ianballou

@sbernhard
Copy link
Member Author

Should I rebase everyting to a single commit or is it fine to have multiple commits for specific changes?

@ianballou
Copy link
Member

Should I rebase everyting to a single commit or is it fine to have multiple commits for specific changes?

We can do a squash merge, so it's no problem.

Copy link
Member

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

This looks good to me! Let's wait until branching happens to merge. It should be soon.

@ianballou
Copy link
Member

@sbernhard I'm ready to merge this, but GH is complaining about conflicts. Mind updating them? The conflicts might relate to the recent branching.

@sbernhard
Copy link
Member Author

Done @ianballou

@ianballou ianballou merged commit c32d9ac into Katello:master Nov 13, 2024
24 of 27 checks passed
@quba42 quba42 deleted the fix_37929 branch November 14, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants