Skip to content

bumps (library-go)#153

Merged
openshift-merge-robot merged 7 commits intoopenshift:masterfrom
p0lyn0mial:pick-up-tcp-timeout
Nov 3, 2020
Merged

bumps (library-go)#153
openshift-merge-robot merged 7 commits intoopenshift:masterfrom
p0lyn0mial:pick-up-tcp-timeout

Conversation

@p0lyn0mial
Copy link
Contributor

@p0lyn0mial p0lyn0mial commented Nov 2, 2020

picks up openshift/library-go#926 which sets TCP_USER_TIMEOUT socket option for KAS - OAS

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2020
@p0lyn0mial p0lyn0mial mentioned this pull request Nov 2, 2020
Closed
p0lyn0mial
p0lyn0mial commented Nov 2, 2020
View reviewed changes
pkg/image/apis/image/dockerpre012/conversion.go Outdated
// Convert docker client object to internal object, but only when this package is included
func Convert_dockerpre012_ImagePre_012_to_api_DockerImage(in *dockerpre012.ImagePre012, out *newer.DockerImage, s conversion.Scope) error {
if err := s.Convert(in.Config, &out.Config); err != nil {
if err := s.Convert(in.Config, &out.Config, conversion.DestFromSource); err != nil {
Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Nov 2, 2020

Choose a reason for hiding this comment

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

I think that conversion.DestFromSource was the default.

Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Nov 2, 2020

Choose a reason for hiding this comment

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

actually - https://github.com/openshift/openshift-apiserver/pull/150/files

p0lyn0mial
p0lyn0mial commented Nov 2, 2020
View reviewed changes
pkg/image/apis/image/v1/zz_generated.conversion.go
out.DockerImageReference = in.DockerImageReference
// TODO: Inefficient conversion - can we improve it?
if err := s.Convert(&in.DockerImageMetadata, &out.DockerImageMetadata); err != nil {
if err := s.Convert(&in.DockerImageMetadata, &out.DockerImageMetadata, 0); err != nil {
Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Nov 2, 2020

Choose a reason for hiding this comment

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

automation set it to conversion.DestFromSource as well.

@p0lyn0mial
Copy link
Contributor Author

p0lyn0mial commented Nov 2, 2020

requires openshift/api#776

sttts
sttts reviewed Nov 3, 2020
View reviewed changes
pkg/image/apis/image/v1/conversion.go
}
out.Tags = make([]v1.TagReference, 0, 0)
return s.Convert(&in.Tags, &out.Tags)
return s.Convert(&in.Tags, &out.Tags, conversion.DestFromSource)
Copy link
Contributor

@sttts sttts Nov 3, 2020

Choose a reason for hiding this comment

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

what do these mean?

Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Nov 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

these were removed in https://github.com/openshift/openshift-apiserver/pull/150/files#diff-476a993839cca71a75d722226b54f2a2e7560296678290d40054fa9b009bfef3L14

my pr just reverts these changes as they were not meant to be merged.

Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Nov 3, 2020

Choose a reason for hiding this comment

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

we should revert https://github.com/openshift/openshift-apiserver/pull/150/files#diff-bc324103be9275286883e8cb4282105288549a542d171c6d9abb4910475f5f92L531 as well

Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Nov 3, 2020

Choose a reason for hiding this comment

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

done.

sttts
sttts reviewed Nov 3, 2020
View reviewed changes
hack/openapi-violation.list
API rule violation: list_type_missing,github.com/openshift/api/oauth/v1,OAuthClient,ScopeRestrictions
API rule violation: list_type_missing,github.com/openshift/api/oauth/v1,OAuthClientAuthorization,Scopes
API rule violation: list_type_missing,github.com/openshift/api/oauth/v1,ScopeRestriction,ExactValues
API rule violation: list_type_missing,github.com/openshift/api/oauth/v1,UserOAuthAccessToken,Scopes
Copy link
Contributor

@sttts sttts Nov 3, 2020

Choose a reason for hiding this comment

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

@marun see here. This is a good start. We should not have these violations.

@sttts
Copy link
Contributor

sttts commented Nov 3, 2020

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 3, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p0lyn0mial, sttts

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

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 3, 2020
@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Nov 3, 2020

@p0lyn0mial: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-cmd 3243401 link /test e2e-cmd

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 3, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 3, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5bb8070 into openshift:master Nov 3, 2020
deads2k added a commit to deads2k/openshift-apiserver that referenced this pull request Nov 5, 2020
@deads2k
Revert "Merge pull request openshift#153 from p0lyn0mial/pick-up-tcp-…
c130146 
…timeout"

This reverts commit 5bb8070, reversing
changes made to 033ead3.
@deads2k deads2k mentioned this pull request Nov 5, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mfojtik mfojtik Awaiting requested review from mfojtik

1 more reviewer

@sttts sttts sttts left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sttts sttts

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@p0lyn0mial @sttts @openshift-ci-robot @openshift-merge-robot @openshift-bot