Skip to content

Conversation

michaelhtm
Copy link
Member

Issue #2641

Description of changes:
Currently we have been ignoring ResolveReferences errors for all
adoptions and readOnly.

Ignoring the error for adopt-or-create is an antipattern, since we want
to ensure users are aware of these issues.

In the case of adopt, since the spec will fully be rewritten, we would
not need to resolve references at all.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ack-prow ack-prow bot requested review from knottnt and rushmash91 September 24, 2025 19:09
@ack-prow ack-prow bot added the approved label Sep 24, 2025
Currently we have been ignoring `ResolveReferences` errors for all
adoptions and readOnly.

Ignoring the error for `adopt-or-create` is an antipattern, since we want
to ensure users are aware of these issues.

In the case of `adopt`, since the spec will fully be rewritten, we would
not need to resolve references at all.
@michaelhtm michaelhtm force-pushed the fix/reportreferenceserrorsforadoptorcreate branch from 46bb7a2 to b59d658 Compare September 24, 2025 19:36
@rushmash91
Copy link
Member

thanks @michaelhtm !
/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2025
Copy link

ack-prow bot commented Sep 24, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michaelhtm, rushmash91

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:
  • OWNERS [michaelhtm,rushmash91]

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

@rushmash91
Copy link
Member

/retest

6 similar comments
@michaelhtm
Copy link
Member Author

/retest

@michaelhtm
Copy link
Member Author

/retest

@michaelhtm
Copy link
Member Author

/retest

@knottnt
Copy link

knottnt commented Sep 29, 2025

/retest

@michaelhtm
Copy link
Member Author

/retest

@rushmash91
Copy link
Member

/retest

Copy link

ack-prow bot commented Oct 6, 2025

@michaelhtm: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
verify-attribution b59d658 link false /test verify-attribution
sagemaker-controller-test b59d658 link true /test sagemaker-controller-test

Full PR test history. Your PR dashboard.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants