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

An exception should be thrown if a configured feature filter has not been registered. #13

Closed
jimmyca15 opened this issue Sep 4, 2019 · 6 comments · Fixed by #50
Closed

Comments

@jimmyca15
Copy link
Member

If a feature is configured to be enabled for a certain feature filter, the feature management pipeline should throw if that feature is being evaluated and the feature filter can not be found. Without this behavior it is very hard to track down the fact that a feature filter has been excluded from being registered. For those who intend for this behavior and depend on an exception not being thrown, this could be configurable via feature management options.

codapus-co added a commit to codapus-co/FeatureManagement-Dotnet that referenced this issue Dec 5, 2019
…r and the feature hasn't been registered, an exception will be thrown when the feature will be evaluated. (microsoft#13)
@jimmyca15
Copy link
Member Author

jimmyca15 commented Dec 18, 2019

The initial design is that exceptions are not thrown if feature filters are specified for a feature and not registered in the application. We have multiple users already using this default functionality. I believe the best approach would be to keep the default behavior and to instead expose the option such that a developer can trigger the exceptions to throw.

@jimmyca15
Copy link
Member Author

@zhenlan @MSGaryWang any preferences either way?

@codapus-co
Copy link
Contributor

@jimmyca15 what you are proposing makes totally sense, regarding that multiple users already using the default functionality. I will wait for @zhenlan and @MSGaryWang comments before making any changes.

@zhenlan
Copy link
Member

zhenlan commented Dec 19, 2019

The initial design is that exceptions are not thrown if feature filters are specified for a feature and not registered in the application.

Is this to allow users to add new filters to existing feature flags in App Configuration before updating/redeploying the application? This doesn't sound like a good practice we would recommend for users to do in production.

I'm not sure I like the silent failure here. I need to understand better why the existing behavior is important for some users (aside from it's a breaking change). If that's indeed the case, I think we should still throw by default and provide an option to suppress it.

@jimmyca15
Copy link
Member Author

@zhenlan you raise a good point. Promoting that as default doesn't lead to best production practices. We should encourage deterministic feature configuration. Going forward switching the default behavior to throw for unregistered filters sounds like a better option. I think it is best for us to take this change now.

@codapus-co
Copy link
Contributor

The existing behavior is important for the users only because the change is a breaking one and for no other reason. The deterministic feature configuration (best production practice) is more important than not making a breaking change. especially at the current state where FeatureManagement is still in preview.

jimmyca15 added a commit that referenced this issue Feb 24, 2020
…ters. (#50)

* If a feature is configured to be enabled for a specific feature filter and the feature hasn't been registered, an exception will be thrown when the feature will be evaluated. (#13)

* Renamed option for ignoring missing filters. Modified exception type thrown when a filter is missing.

* Fixed typo.

Co-authored-by: Sotiris Zacharopoulos <codapus-co@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants