Skip to content

Fix SpringExampleController so it doesn't crash on startup #1686

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

Merged
merged 1 commit into from
May 6, 2021

Conversation

dsyer
Copy link

@dsyer dsyer commented May 6, 2021

This fix is masking another problem somewhere else, but at least it gets
the apps working while we investigate.

Fixes #1684 (temporarily at least). Also rebased on #1685.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 6, 2021
@dsyer dsyer force-pushed the shared-informer branch from 7a13d15 to 2a46bb1 Compare May 6, 2021 07:39
@dsyer
Copy link
Author

dsyer commented May 6, 2021

I understand the problem. It was actually introduced in ce7bc1b when we fixed the post processor. The old version used to instantiate beans (very bad) but then it had the concrete runtime type to inspect. The new version only has the declared bean type, which in these samples is SharedInformerFactory, and that has no interesting annotations. The fix in this PR just ensures that the declared type has annotations that can be processed.

/assign @brendandburns

@dsyer
Copy link
Author

dsyer commented May 6, 2021

/assign @brendandburns

Copy link
Member

@yue9944882 yue9944882 left a comment

Choose a reason for hiding this comment

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

LGTM, @dsyer can you rebase onto the latest master so we can have a cleaner tree structure (b/c github does non-ff merge by default)

This fix is masking another problem somewhere else, but at least
it gets the apps working while we investigate.

Signed-off-by: Dave Syer <[email protected]>
@dsyer dsyer force-pushed the shared-informer branch from 2a46bb1 to b61f2e9 Compare May 6, 2021 08:06
@dsyer
Copy link
Author

dsyer commented May 6, 2021

Github can do "rebase on merge". Anyway I rebased manually and pushed it back.

Copy link
Member

@yue9944882 yue9944882 left a comment

Choose a reason for hiding this comment

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

Github can do "rebase on merge". Anyway I rebased manually and pushed it back.

thanks @dsyer, we're merging via the bot which i'm not sure capable of doing that

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 6, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dsyer, yue9944882

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2021
@k8s-ci-robot k8s-ci-robot merged commit 7f69d7f into kubernetes-client:master May 6, 2021
@dsyer dsyer deleted the shared-informer branch May 10, 2021 08:07
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpringControllerExample fails to start
4 participants