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

NamespaceSelector updates triggering evaluations #157

Conversation

JustinKuli
Copy link
Member

@JustinKuli JustinKuli commented Aug 8, 2023

This approach uses the new feature in github.com/stolostron/kubernetes-dependency-watches, but it comes with some downsides (compare to #158):

  • that library doesn't help if the NamespaceSelector only uses Include and Exclude
  • extra evaluations can be triggered when there aren't actually updates

This implementation also uses a channel for communication as opposed to a shared map that would likely require a mutex, but I don't know if this approach is actually better.

This also tries to use channels to communicate, as opposed to a map that
would need a mutex... I don't know if it's really better, it feels out
of place. The downsides to this approach are noted in the last 2 new
tests.
@openshift-ci
Copy link

openshift-ci bot commented Aug 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JustinKuli

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

@openshift-ci
Copy link

openshift-ci bot commented Aug 8, 2023

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

  • 6e32ff4 NS Selector tests
  • ac16313 DRAFT: evalauting NS-Selector with dep-watches

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.

@mprahl
Copy link
Member

mprahl commented Aug 8, 2023

As of now, I prefer the other approach.

@JustinKuli
Copy link
Member Author

Closed in favor of #158

@JustinKuli JustinKuli closed this Aug 8, 2023
@JustinKuli JustinKuli deleted the 6428-ns-evaluations branch July 25, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants