Skip to content
This repository has been archived by the owner on Oct 10, 2018. It is now read-only.

Allow providing dynamic routing callback #43

Closed
mkosieradzki opened this issue Feb 22, 2017 · 8 comments
Closed

Allow providing dynamic routing callback #43

mkosieradzki opened this issue Feb 22, 2017 · 8 comments

Comments

@mkosieradzki
Copy link
Contributor

It would be great, if I could provide a callback that will make decision where to route request. Without this I need to fork entire middleware (because my routing rules change during runtime).

If that's acceptable I would be happy to make a pull request.

@Tratcher
Copy link
Member

Note we're not actively developing or shipping this component. We know the configuration and routing need quite a bit of design, but don't expect to do so any time soon. I'd recommend creating your own fork to experiment with for now.

@mkosieradzki
Copy link
Contributor Author

I've actually copied code from this component long time ago and it is extremely useful. I would like to put some effort into it based on my experiences with proxying for Service Fabric, but it only makes sense if I it will receive a fighting chance to be integrated :).

IMO This component is really not so far away from being production ready.

@mkosieradzki
Copy link
Contributor Author

@Tratcher Quick question: Can I make ProxyOptions immutable? (It would be part of larger refactoring). Or is there some non-optional convention requiring public setters on properties on middleware options classes.

@Tratcher
Copy link
Member

I don't follow, what's the point of Options that you can't change?

@mkosieradzki
Copy link
Contributor Author

@Tratcher You configure all options using constructor and construct object that is immutable. It's cleaner because no one can modify options when middleware pipeline is actually running (what can have unpredictable effects and is far from recommendations), and also allows to create some options constructor overloads for different scenarios.

@Tratcher
Copy link
Member

Ah. We do that for most classes, (e.g. middleware), but not for Options. Not all of the settings may be known at creation, you create the Options and then keep configuring it until you're ready. See this pattern:
https://github.com/aspnet/BasicMiddleware/blob/dev/samples/ResponseCompressionSample/Startup.cs#L23

@mkosieradzki
Copy link
Contributor Author

I like this pattern with lambda as well it limits the scope where you can modify options. OK. I will have some looks on pre-existing middlewares, but I will definitely refactor the current implementation as I don't like it.

I am sorry to take your time (but this discussion will save a lot of it during code-review ;) ).

Would it be acceptable if I make ProxyOptions class private implementation detail and provide some convenient overloads for RunProxy

For example:
RunProxy(option1, option2, option3 = 80)
RunProxy(option1, option4)

And if complex configuration is needed (I don't expect it yet):
RunProxy(options => {
options.RegisterRoute(...);
options.RegisterRoute(...)
})

This was referenced Feb 23, 2017
@Tratcher
Copy link
Member

Having RunProxy overloads for common options is fine, but keep them simple.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants