-
Notifications
You must be signed in to change notification settings - Fork 455
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
Discussion on deprecated APIs for removal in future versions #338
Comments
I do believe most of the XML configuration can be removed. I think XML removes the compiler from type safety resolution and promotes these problems to the runtime environment. So I would think any XML based config needs to hit the road and go to that XML heaven in the sky. |
It also seems like we should get rid of parent/child containers. I think what you need to do is read @hammett's original post about this: http://hammett.castleproject.org/?p=296 @kkozmic tried to reason with this implementation but I do believe it creates more problems than what the micokernel already solves:
We should really be using: |
I hope that we can treat this as an opportunity to discuss things that should not be here apart from mere obsolescence, parent/child containers appears to be a big after trawling through all the old issues tonight. This is steering users down the wrong path with the wrong expectations. A major version increment is hopefully where we get to discuss things like this and make things easier for people. It is my hope that there is more that people would like see more come out of this that makes Windsor easier to use. |
Yes as you've discovered they are a source of many problems because the rules around them still aren't defined. Reading the comments on @kkozmic blog posts does make me realise that today child containers are really the only solution for some people's requirements. A handler selector doesn't let you register different components without having them always registered, a sub dependency resolver would be a painful way to perform this task, it means if you were doing WinForms (or any long lived app) your root container has common stuff and as you enter a specific module (doesn't even need to be a plugin) of the application its child container gets created for use and destroyed on closing, you couldn't really do that without child containers. Having a look at Autofac they don't appear to have child containers but creating an Autofac lifetime scope does provide an inner container builder where you can register more types that don't infect the parent scope. I think we could probably do the same and allow the scoped lifestyle to support registrations, singletons would survive in the container, scoped lifestyle components would be released on scope disposal as they are now, transient is obvious, and all other lifestyles would behave like they were resolved outside the scope. Thinking through many of the requirements in those blog comments this would handle most cases. Windsor has been used in long lived programs (GUI and server) for a long time and will continue to be, web apps are only one use case. What we really need is for users to get involved so we can learn about people's use cases. |
From that list, we use I'm a little confused by the deprecation messages on those to be honest, perhaps we've been using them wrong? Are there alternatives? Now regarding child containers: we're using them in an application that has a plugin style architecture, where each plugin can create and attach its own child container to the main container. A plugin represents a module, and whenever a module is opened, the child container is created and the plugin registers its own components (possibly overriding some main container's defaults). When the module is closed by the user, the child container is disposed and there should be no trace left of any of the module's components. This works quite nicely as the plugins can re-use defaults from the main container for e.g. UI stuff, but they can modify behavior at the same time. Regarding #324: this 'bug' is only important to me because we currently have to manually One other thing that caused some confusion: resolving transients using |
The members you mention are not deprecated and look like they will live on in the FromDescriptor. The BasedOn methods here are in the BasedOnDescriptor to stop the fluent interface from being re-entrant and resetting. I think you might be OK. Parent/Child containers look like they need proper rules hashed out. Coming up with a language that could concisely define this would not be an easy task. We should try lead up to it by discussing it more. I have some additional thoughts around singletons. For example, if it is registered in the parent container and already resolved then it should stand to reason they would survive the disposal of the child container however if they are registered in the child container then surely it would make sense that it is not resolvable via the parent and would not survive the child containers disposal. This fundamentally alters how a singleton would behave. Furthermore disposables that are transient's and then consumed by singleton's should probably be disallowed altogether in parent containers, in child containers the singleton would be tracked and are assumed to be shorter lived so this would potentially be viable as the child container would eventually get disposed. Our rules need to factor in where the component was registered, what the intended lifestyle was and how it behaves or is tracked in a child container and then what happens to it when the container is disposed. No easy task! :) The divergent side of my brain is thinking do we even need this? What we are really after is a way creating shorter lived containers that serve as plugins with re-usable setups. I am a massive fan of using installers. These allow you to create and share common container setups and common installers can be consumed by specialised installers for child scenarios. Simply creating a new container with distinctions between how it is used and how installers are composed together might actually be a simpler solution to the problem. This also avoids sending us down the path of trying to support mutated lifestyles. Much easier! I still believe we should get rid of this feature, I can solve it by using composition at the installer level. |
Usage of child containers It rarely about reusing container set up but having access to parent container instances. |
Still struggling to understand why this is a "must have" feature. Parent/Child Scenario ParentForm <- Registered in parent container, instantiated. Click resolves through N possible ChildForm's. ChildForm <- Registered in child container, to be instantiated but requires a reference to ParentForm. ParentIWindsorInstaller -> Holds reference to parent container. Preferably protected static named ChildIWindsorInstaller -> Inherits ParentIWindsorInstaller and then will have access to both ParentContainer/ChildContainer. Then add a custom SubDependencyResolver to the child that checks resolution between the parent and child containers because you already have a reference to both. Might be missing something obvious here but this is potentially how I would do it. |
Wouldn't you run into even more lifetime problems doing it that way? It would be good to hear some more detail from @reijerh how their application works, what type of services they register in the child container. |
I am thinking less problems because lifestyles are more deterministic and don't have to mutate their behaviour across containers with blended release policies for tracked items as I mentioned earlier. I am happy to be wrong on this. Just thinking out loud really.
Yes, I would love to know more and even what I am proposing is possible. |
Parent/Child containers are used by other framework as a container integration thing. I remember NServiceBus using/doing something like that. But it is a very naive and limited implementation, definitely needs some love. Imo the whole component burden thing should be removed. Everytime people discover that I was a castle contributor they tell me a war story about a production memory leak caused by it. In the end people set the NoTrackingReleasePolicy and move on not caring about it. The xml config stuff I thinl that is very usefull for store some app properties. I usually have all the have lifting done by the fluent api, and leave some settings be setup on xml. And not sure if its the right place for it, but one thing that hammett and I discussed in the last years, is that the openness for modification is maybe a bad thing. Most of people configure the container at the startup, and never changes it. But this assumption (that it can be modified at any time in the app lifetime) feeds a lot of simple mistakes (that could be detected with simple checks). |
I went through each module's installer, and as far as I can tell most registrations could just as well be done in the main container. The only argument I could think of against doing this is that it 'pollutes' the main container for as long as the app lives, even though the module was already closed. There is one thing that I think would be tricky to do without child containers though: overriding implementations. As an example, we have an Some more modern modules have adapted a different way of showing progress due to a new UI design, and so they implement their own Repositories and ViewModels are other examples of things we override in modules. So I'm guessing this scenario might be implemented using some other Windsor extension point? BTW, about burdens: I had to use |
This is the main use case for us. Root parent container, then child container per "open document" which gets disposed when the document is closed. This pattern takes care of dependency lifetimes very nicely. We've tried using scoped lifetimes for this, but there are limitations in this mechanism which aren't present in parent/child containers, one which @reijerh mentioned: no override (which we use on some document types). We've also extended parent/child containers to handle this case more flexibly and robustly than vanilla Castle. I'd be interested in helping work through lifetime scope issues to get a solid answer for v5.0. |
@codekaizen - I am interested to know more about your lifestyle configuration here, can you shed more light on how these are setup? We just had a case with transient disposables registered in the parent were not being disposed when resolved through the child container and then disposing the child container in #324. @jonorossi has already said this breaks the resolve/release rule but burden tracking should perhaps be able to do this? @hconceicao has already stated that burden tracking is probably wrong and should be removed. Would be good to hear about a |
If anyone is interested, I have an implementation of a plugin architecture which use a subdependency resolver and by completely avoiding the parent/child api's of windsor. It uses a https://github.com/fir3pho3nixx/Windsor.Plugins Look forward to your feedback. |
@fir3pho3nixx - interesting. My main app is a web app, but we're starting to have some things being done on the client-side on a Windows PC. Our bridge for that is registering a protocol handler of the form Multiple Rather than launch a new instance of the EXE every time I'd like to make a long-lived but simple winforms/wpf app that sits in the taskbar. Each invocation from the server launches a stub exe which finds the running client app and gives it the command. So... I'll have one root Windsor container in that app for things like registering a logger, a factory to give me a HttpClient instance and some other useful things. But each invocation - sometimes of the same action type - should have its own "container" scope much like you were describing your view models. I occasionally would need to override something that the parent container would normally provide. I'd also like to dispose of everything in the child container as a sort of 'unit of work' when the user action is done. I suspect I might be able to do some of this with some sort of bound & scoped lifetimes but it's just something I've never quite got my head around as the child container approach seems more straightforward. The child container, as far as I'm concerned, can be a form of handle which defines the scope and lifetime. Registering things in there is the extra bit which your plugin thing fixes, as well as making the concept more simple. Am I on the right track with my thinking? I implemented a form of this with Windsor already but ran into trouble with, from memory, this scenario. I had a transient registered in the parent container. The transient took in a dependency. That dependency however wasn't available until I was actually invoking an action - so I'd explicitly register that dependency in the child container (in this case it was a specific instance of an object). I'd then try to resolve that parent-registered transient from the child container. If memory serves correctly, Windsor didn't like that. |
Yes, the idea here is that things fallback to a vanilla or "well known" understanding of how lifestyles are managed in a single container and removes the complexity of who registered what where and how that cascades and mutates between child/parent resolves. It also leverages object tracking to the best of it's ability for transients/singletons. I have outlined my assumptions in this section.
My pattern "kind of" solves this scenario but I do have a few questions/assumptions to try and understand it better. The problem: Given: We want to resolve Transient A from child container. ParentContainer -> Registers Transient A (requiring Dependency B, not registered) -> Handler.State == WaitingDependency. Therefore always potentially a misconfigured component in parent container. ChildContainer -> Registers Dependency B -> Handler.State == Valid. Resolvable but cannot contribute to Transient A's constructor because this is in the parent container. Questions:
In summary, if this is never resolved through the parent container it would benefit greatly from being pushed down the the child containers. You could also probably apply some clever polymorphism in your installers to make this happen by convention. Resolving Transient A from the parent container is a completely different story, this would simply involve no overriding registrations in your child container. |
I think it's also worth deprecating EnvironmentInfo (https://github.com/Huddle/Castle.Windsor/blob/master/src/Castle.Windsor/IEnvironmentInfo.cs). It's rarely used (even though I have used it extensively in the past!) it's really no longer necessary. |
I have updated this issue with some of the topics mentioned here. All marked as |
Linking discussion: #346 and adding it to the issue description. |
@fir3pho3nixx
Hi. Im Windsor's long time user and I strongly disagree with this. Fluent registration in code is cool and I use it for most things but XML config still has its use. Its invaluable for storing various setting for components - things which are so low level I don't want to put it into app\web config but still have some way to change it without building and redeploying my app. It can do things app.config cant as arrays etc. |
@fir3pho3nixx
You can use XML just for
Yep but this is the case with any external configuration be it xml, JSON or any other store. Its trade-of between safety (compile time checking) and flexibility (ability to change behavior without compilation and deployment). @kenegozi You've mentioned life-cycle management as one of the container's responsibilities. Well, creation of components and setting up dependencies is part of the life-cycle. In my book, support for setting up dependencies in external configuration, support for powerful type conversions (+ ability to provide your own type converter if needed) is infrastructure concern. Its addressed very well in Windsor and makes my life so much easier because its easy, powerful and "just works". In the past when I was comparing DI implementations in net, it was one of the selling points of Windsor for me and its removal would be huge step back imho Is there any discussion here about how exactly this part of Windsor is "holding it back"? Or are we just comparing our "holistic views" on how all the apps should be developed ? |
@Liwoj this discussion is exactly that, a discussion to gauge what sort of stuff we could get rid of because Windsor is huge with so few people contributing to maintaining it, so thanks for joining this discussion as it wasn't specially about XML configuration or any one thing but the fact that there are obsolete things all over the place. @fir3pho3nixx edited the issue description by collating a list from the comments that makes this issue look more like a todo list than just a discussion thread, but he has also included "Not a thing for v5" against "XML config in general". Between us there are mixed opinions on XML config, but I believe XML config still has a use case (including heaps of history usage that isn't just going to magically rewrite itself) exactly as you describe and I suspect if we do anything in the medium term it'll be to de-emphasise XML config in our docs to encourage usage only when C# can't be used. We also have work to do to make sure all facilities can be configured easily by code, even the just recently resurrected Quartz.NET facility heavily uses XML config and doesn't have a C# interface (other than building the XML configuration object) to configure it, like some other facilities. @Liwoj I highly recommend watching the Windsor repo to get notifications, it is low volume and we need more users involved in discussions on how to move forward, what should change, what should stay the same, even if you don't have time to contribute code. 9 participants on this thread, surely there are more than 9 Windsor users out there. |
I should add that I don't see XML config as holding Windsor back like a bunch of other areas where things are sort of broken and will needing breaking changes to fix, areas where users are running into defect after defect like factory facility and child containers. In regards to XML config, what we really need to establish first is an alternative to XML config at minimum via samples showing how users can do that before considering deprecating it. I feel that removing XML config would cut Windsor at it's knees for many users since it is such a long standing core feature from day one, these users will struggle to migrate big apps and likely just stay with an old version of the library and then never contribute. I agree with @kenegozi that environment variables are the way most applications are configured today, not a big XML document (even though it can resolve environment variables). Most of the other features we've been removing have been long marked obsolete or thought of that way for a long time, so I think we should give users the courtesy of some time if this is to happen here, i.e. deprecate, then later remove. |
@jonorossi : Indeed, by holding back I refer to the added complexity of maintaining an ever growing matrix of features that need to play nicely together. Even in a 100% funded project it is not an easy task, let alone volunteer run OSS. And removing the support for such a feature is definitely a breaking change. Question is how many people how are users of XML config are also likely to upgrade to a possible no-xml-anymore vNext version of Windsor. There could still be made a layer by which a library, external to Windsor's core, can read the XML config and call windsor's well published registration APIs. This would inverse the coupling, and declare programmatic configuration as core and XML configuration as wrapper. @Liwoj Can't agree more. Lifecycle management and general extensibility is what sets Windsor apart from other .NET containers. What I argue is that bundling general-purpose config into the container is mixing up concerns where they should not belong. |
@jonorossi - sorry I edited the issue, was trying to create a quick reference for agreed items. Avoiding having to read through the entire issue. An example where xml config has been used inappropriately is in type factories. Component registrations plug things in, only so interceptors can reference it later. Not sure this is a good thing. |
@kenegozi exactly, we first need to make sure everything we are supporting actually has a programmatic interface so we can flip the direction. I guess since external configuration was one of the ticket features of Windsor over MicroKernel it has really driven a lot of the past design decisions. I'm definitely not against moving away from XML config, but in the context of this issue I don't think version 5 is the place as we aren't at the point. Windsor's external configuration is always more powerful than most people realise at first, there are a lot of extended features (conditionals, include, etc) in there so not always just a quick change to C#. What do you guys think of a Windsor external configuration compat library (another assembly/package we maintain) that would force use of the public API to perform registration? Most/all of
@fir3pho3nixx no problem, I was just pointing out this isn't a formal todo list even if it appeared a little like that. |
How much of this do we want to action before close? AllTypes/AllTypesOf has been addressed. https://github.com/castleproject/Windsor/blob/master/CHANGELOG.md I would still like to address: #338 (comment). |
Sorry @hconceicao and @jnm2. I think you guys raised some hot issues. You are not being excluded, I personally feel your pain because I run this container in production everywhere I go. We just don't have the time to build this into V5. Please raise new issues for anything we missed here. Thank you for your input. Kindest respect. |
Since this is just a discussion thread rather than a specific action item, maybe we just rename the title to remove v5, there is still quite a lot of info we've discussed that would be good to capture in proper separate issues in the short to medium term. |
@fir3pho3nixx Sure, no problem! The only issue I remember bringing up right now already has an open issue, #346. |
I am also cool with that. |
I second this. My solution uses a mix of fluent registration in code and XML registration. Fluent registration using convention is cool because I can add implement new services and components and they are registered automatically for me. However, XML registration is cool too. The ability to easily register items of an array in the specific order is useful. In addition, my customers appreciate the fact that they can plug in some custom implementations themselves: "hey, you need to implement this interface and then you 'just' need to add it to that xml file!" Btw, my solution also uses child containers because it needs to register components that vary on a request by request basis. |
We have also relied on xml configuration for a long time as a means to configure our applications at deployment time but we have migrated to a very simple
I'm in favor of deprecating all of the xml config stuff because moving to a custom json config was really easy, gave us a lot more flexibility and reduced the number of dimensions for app configuration to only those that we wanted to support. My preference would be an easy integration path for asp.net core json config so you can get all the other great benefits of using that such as easy environment variable support. |
For god's sake, please keep XML config support. It IS working well and being permissive is a great thing for advanced users. Some can favor json-based, but that doesn't mean json is anything better than XML. The greatest thing about Windsor is that it is very matured with so many features, and that also means it has a large user bases, some of them are enterprise applications that heavily use every features that Windsor can offer. Before saying that moving from XML to JSON is easy, please consider the fact that it might not always be easy for everyone, depending on how advanced one uses XML configuration. "Really easy" might mean hundreds of hours of coding and testing. Today I tried to upgrade my application to a latest release and found out that LifestyleType.PerWebRequest was removed (Yup, I'm a bit late to the party, but my case could represent what some developers of enterprise applications have to do.) The lifestyle was removed and it is recommended that LifeStyle.PerWebRequest() should be used. What doesn't work for me is that my code can run in two different hosting environments, and one uses Transient/Singleton while the other uses PerWebRequest. Thus, I passed a LifestyleType to the installer which is obviously broken now because the PerWebRequest lifestyle disappeared. Maybe the one you removed it thought changing to PerWebRequest() could be easy? `
` |
@thuannguy Firstly this issue is just a discussion to get an idea of where users sit, it is literally tagged and titled discussion, couldn't be any clearer. Windsor might have a lot of features but who maintains all those features as platforms/runtimes change as the years go by. The discussion around XML config was heading towards having it as an optional add on in the way the MicroKernel was extended with XML config to create Windsor (they used to be 2 separate projects many years ago), not being removed completely. Windsor has been built by volunteer contributors, and it is severely lacking them in recent years. Regarding |
@jonorossi I'm all in for making it optional, as long as existing XML features will still be working well. Some of the comments suggested to deprecate it which is a dangerous move from my point of view. Therefore, I had to raise my concern. Regarding LifestyleType.PerWebRequest, yep, as you said it is the reason I felt the need to provide input here because I don't want the same thing happens for such an important and powerful feature. My example is all about things might not be so easy for other people. While I can understand the problem of lacking of contributors. I'm not here to criticize anyone because my cheese was moved! Instead, I'm grateful to you guys for maintaining the library that has benefited me a lot. So thank you for that 😄 I don't buy the "if you don't want change then don't upgrade" advice. Changes are one thing, being left behind because of a road-blocker change is another thing. |
@thuannguy I'd strongly recommend watching the Windsor repo on GitHub and participating in discussions including reviewing pull requests to ensure you like the direction of the project. It is an extremely low traffic project at the moment and the only way to make sure you voice is heard is to be involved. |
@jonorossi I subscribed to the watch list as you recommended. I will try to get involved as much as possible. |
With: Parameters Now removed, what are the alternatives? Or should I just stick to v4 for this legacy app? |
@phillip-haydon those methods that were removed had all been marked obsolete with alternatives in the reason text for many years, |
@jonorossi - I know, I'm not complaining, just wanting to know what direction I need to go in. This is in a legacy app that's compiled and touched once every couple of months, someone decided that it would be a good idea to write a wrapper and a service locator around Windsor. So today I dived in to do a dependency bump and came to realise the APIs were deprecated, so looked at some of the documentation. This section seems to be out of date. So I found this thread, and decided to ask :) DependsOn is working fine though, I've moved from the old APIs to DependsOn and it works without issue. Thanks. |
@phillip-haydon good to hear you sorted it out, don't we all love legacy apps 😉. Don't worry I didn't think you were complaining, I agree the docs can sometimes be lacking, it looks like you found one that got missed, any chance for a pull request to fix that page. |
Will sort a PR out tomorrow :D already turned computer off for the night. |
The community needs to discuss what obsolete public public API's can be removed for Windsor V5, including anything that might no longer be useful or could be replaced with something from the framework.
Anything that is identified as "generally agreed" will be dealt with in a PR which should be linked here.
Below are a list of obsolescent items found and their current status.
Logging Facility:
LoggerImplementation
enum and theloggingApi
XML propertyXML config in general (Not a thing for v5, @fir3pho3nixx)Parent/Child containers (Still in flight, @fir3pho3nixx)
Burden Tracking (Still in flight, @hconceicao)
IEnvironmentInfo (Still in flight, @jonnii)
Deprecating Resolve(Type, IDictionary) in favor of IReadOnlyDictionary<string, object> (Still in flight, @jnm2)
Obsolescense in general (Generally agreed)
ActAsParametersServiceOverridesPropertiesBasedOn'T
Where
CustomLifestyleAttribute
NoTrackingReleasePolicy
AllTypes && AllTypesOf
DefaultKernel_ObsoleteAPIWindsorContainer_ObsoleteAPIThe text was updated successfully, but these errors were encountered: