-
Notifications
You must be signed in to change notification settings - Fork 9
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
ResourceHandler does not provide defaultLogger #4
Comments
Could you use this for now?
If you want to generate more Resources right now, I could change the generation to include these lines. If you just need it on one or two, or on already generated Resources, you can copy it in. |
Oh yeah, that's okay. 👍 Thank you for your fast support! 😄 |
@Florian-Schoenherr Has the code generation been fixed such that it supports your suggestion? |
@VaughnVernon I kept this open mainly because #5 was not the most optimal solution. We could close, but I think there is still some misconception of The fix I applied is just "here, have another logger". |
Thanks, @Florian-Schoenherr @danilo-ambrosio When you have an chance, please address this. I don't know where the problem is:
The problem is that when |
I tried removing |
If you remove |
That did not work. Maybe it was something specific to schemata, my guess is that xoom's |
You're right, @Florian-Schoenherr.
What do you think? @VaughnVernon @Florian-Schoenherr |
@danilo-ambrosio Do you mean a marker class/interface? I prefer not to use one. However, you could put a meaningful method on the interface:
Now overriding |
Well, I know why marker-classes are not so nice. Also, aren't annotations supposed to be used for marking (too)? The interface with @danilo-ambrosio do we need anything else on the marker class/interface? |
@Florian-Schoenherr @danilo-ambrosio It could be an abstract base that has a constructor that requires the
|
We could use annotations but we still need to ensure some methods implementation on resources classes. If we use annotations instead of marker interface/class, then a compile-time validation is required for making sure the right methods are implemented. That takes time to be implemented. So I vote for @VaughnVernon 's suggestion which seems to be simpler. Note that |
@danilo-ambrosio Do you mean injected as in passing a constructor parameter from the generated |
Yes, "injected" means exactly what you've just said, @VaughnVernon . Thanks for clarifying. |
then obviously no annotation 👍 |
@Florian-Schoenherr @VaughnVernon If you get a chance, please review my changes below and let me know if everything is right:
After the review, could @Florian-Schoenherr please update VLINGO/SCHEMATA REST resources by extending |
Works as expected! 👍 |
@danilo-ambrosio @Florian-Schoenherr I am closing this. Note that I have added support for accessing |
It's possible that extending ResourceHandler is improperly generated because that is only supported by "configuration resources" (declared in vlingo-http.properties). For dynamic resources handlers the Stage must be passed in via constructor. With the Stage the dynamic resources handlers can get to anything needed, including the Logger.
/cc @danilo-ambrosio @Florian-Schoenherr
The text was updated successfully, but these errors were encountered: