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

pullthrough is not optional #103

Merged

Conversation

legionus
Copy link
Contributor

I’m not sure pullthrough enabled false is something we should support. In 8 release we’ve never had a reason to trn it off. (c) Clayton Coleman

openshift/origin#19969 (comment)
#101 (comment)

/cc @smarterclayton

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 14, 2018
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 14, 2018
@legionus legionus changed the title [WIP] pullthrough not optional [WIP] pullthrough is not optional Jun 14, 2018
@legionus legionus force-pushed the alweays-pullthrough branch from 81543fa to 29dc0a4 Compare June 14, 2018 09:14
@legionus
Copy link
Contributor Author

/retest

useBlobDescriptorCacheProvider bool,
) (distribution.Namespace, error) {
cfg := &configuration.Configuration{
Server: &configuration.Server{
Addr: "localhost:5000",
},
Pullthrough: &configuration.Pullthrough{
Enabled: pullthrough,
Enabled: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just drop this parameter from the configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmage I want to show warning if someone have it as false. But I can drop the Pullthrough initialization here.

@bparees bparees added this to the v3.11 milestone Jun 14, 2018
@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 14, 2018 via email

@legionus legionus changed the title [WIP] pullthrough is not optional pullthrough is not optional Jun 15, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 15, 2018
@legionus
Copy link
Contributor Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2018
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 20, 2018
Signed-off-by: Gladkov Alexey <[email protected]>
@legionus legionus force-pushed the alweays-pullthrough branch from 29dc0a4 to 8516796 Compare June 21, 2018 07:59
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2018
@legionus
Copy link
Contributor Author

Rebased

@legionus
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 21, 2018
@bparees
Copy link
Contributor

bparees commented Jun 21, 2018

/lgtm
/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, legionus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@legionus
Copy link
Contributor Author

/retest

2 similar comments
@legionus
Copy link
Contributor Author

/retest

@legionus
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit 3d889c8 into openshift:master Jun 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants