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

Changing download_policies #1210

Merged
merged 1 commit into from
Jul 3, 2019
Merged

Conversation

bmbouter
Copy link
Member

@bmbouter bmbouter commented Jul 2, 2019

With pulp/pulpcore@59f3450
the download policy by default only includes 'immediate'.

[noissue]


JWT_PATH = urljoin(BASE_PATH, "jwt/")
Copy link
Contributor

Choose a reason for hiding this comment

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

🥇

Copy link

Choose a reason for hiding this comment

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

We kept this one around since JWT could be added back based on the discussions at the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since then though we removed the JWT entirely so I think we should remove this too.

Copy link

@nixocio nixocio left a comment

Choose a reason for hiding this comment

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

A few tests will be required to be updated with those changes.

Just a few examples, see: test content delivery
download content

@@ -22,13 +22,9 @@

DISTRIBUTION_PATH = urljoin(BASE_PATH, "distributions/")

DOWNLOAD_POLICIES = ("immediate", "on_demand", "streamed")
DOWNLOAD_POLICIES = ("immediate", )
Copy link

Choose a reason for hiding this comment

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

This name can be misleading. Since it is plural. I would suggest:

Suggested change
DOWNLOAD_POLICIES = ("immediate", )
IMMEDIATE_DOWNLOAD_POLICY = ("immediate", )

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 pushed this fix, but what about just making it

IMMEDIATE_POLICY = "immediate" since it's single now.

bmbouter pushed a commit to bmbouter/pulpcore that referenced this pull request Jul 2, 2019
The "lazy" variables are renamed to be "on-demand" to match recent
changes in pulp-smash.

Required PR: pulp/pulp-smash#1210

[noissue]
bmbouter pushed a commit to bmbouter/pulpcore that referenced this pull request Jul 2, 2019
The "lazy" variables are renamed to be "on-demand" to match recent
changes in pulp-smash.

Required PR: pulp/pulp-smash#1210
Required PR: pulp/pulp_file#255

[noissue]
@bmbouter
Copy link
Member Author

bmbouter commented Jul 2, 2019

I pushed PRs to pulpcore and pulp_file with fixes:
pulp/pulpcore#199
pulp/pulp_file#255

nixocio pushed a commit to nixocio/pulp_rpm that referenced this pull request Jul 2, 2019
The names were updated to replace 'lazy' with 'on demand' and also
reflect the recently changed validation in pulpcore.

Required PR: pulp/pulpcore#199
Required PR: pulp/pulp-smash#1210

[noissue]
@bmbouter bmbouter force-pushed the restrict-remotes branch 3 times, most recently from 5c0f301 to e1db5da Compare July 3, 2019 13:35
@@ -2,9 +2,9 @@
"""Constants for Pulp 3 tests."""
from urllib.parse import urljoin

from pulp_smash.api import ( # noqa: F401
from pulp_smash.api import (
Copy link

Choose a reason for hiding this comment

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

Suggested change
from pulp_smash.api import (
from pulp_smash.api import ( # noqa: F401
_P3_TASK_END_STATES as P3_TASK_END_STATES,
)

With pulp/pulpcore@59f3450
the download policy by default only includes 'immediate'.

Required PR: pulp/pulpcore#199
Required PR: pulp/pulp_file#255

[noissue]
@bmbouter bmbouter merged commit 81d88ae into pulp:master Jul 3, 2019
@bmbouter bmbouter deleted the restrict-remotes branch July 3, 2019 15:13
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

3 participants