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

ACC-899 Replace Fabric8 watchers with informers #168

Merged
merged 3 commits into from
Jul 28, 2023
Merged

Conversation

tgeens
Copy link
Contributor

@tgeens tgeens commented Jul 28, 2023

No description provided.

@tgeens tgeens requested a review from a team as a code owner July 28, 2023 10:08
private boolean enabled = false;
private String namespace = "default";
private long resync = 60;
Copy link
Member

Choose a reason for hiding this comment

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

That seems fast? We'll always get a bunch of update events that we need to handle every 60s. The Object.equals will handle it early, but still...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's worse, I forgot the * 1000L part ;-)

the minimum resync interval is 1 sec - which runs fine locally pointing to a real K8s cluster
Why would once every minute not be fine in production ?
(spring cloud k8s uses 30 secs by default)

Copy link
Member

@rschev rschev left a comment

Choose a reason for hiding this comment

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

Approved because even if the 60s is too fast, we can just update the configuration in deployment.

@tgeens tgeens changed the title Replace Fabric8 watchers with informers ACC-899 Replace Fabric8 watchers with informers Jul 28, 2023
@tgeens tgeens enabled auto-merge July 28, 2023 13:16
@tgeens tgeens merged commit ca32acd into main Jul 28, 2023
4 checks passed
@tgeens tgeens deleted the fabric8-use-informers branch July 28, 2023 13:17
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.

2 participants