-
Notifications
You must be signed in to change notification settings - Fork 64
WIP: Hooks for DI frameworks #310 #318
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
Conversation
Thanks @patriot1burke for the PR, really appreciate your work. Just wondering if we can follow a same pattern as spring cloud function on initialize the DI frameworks. This give your maximum flexibility to update/release on your side instead of having a hard dependency on our side. The spring cloud function example can be found https://github.com/spring-cloud/spring-cloud-function/tree/main/spring-cloud-function-samples/function-sample-azure. The are initialize spring application context at the interface at https://github.com/spring-cloud/spring-cloud-function/blob/526e6eebec0777157cc4bf2dfe75a79c4a706f00/spring-cloud-function-adapters/spring-cloud-function-adapter-azure/src/main/java/org/springframework/cloud/function/adapter/azure/FunctionInvoker.java#L84. Please let me know what you think of this approach, it make the function a little bit verbose but remove the hard dependency on us. Thank you. |
@kaibocai Thanks for getting back to me. We could do it that way, but we can have smoother integration. With the approach I'm suggesting, the application developer writing a function class would not have to extend any special class like they do with the Spring example. Quarkus would be initialized automatically and you could use injection annotations (CDI or Spring) directly in the function class providing nice smooth integration with Azure functions. Again, take a look at the sample class I posted which is what the integration would look like if you added this small hook. |
@kaibocai Just trying to improve the user/appdev experience with Azure Functions + Quarkus. I'm sure the Spring guys would love to have this hook as well! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please rebase the latest dev branch.
src/main/java/com/microsoft/azure/functions/worker/broker/JavaMethodExecutor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/azure/functions/worker/broker/JavaMethodExecutor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/azure/functions/worker/broker/JavaMethodExecutor.java
Outdated
Show resolved
Hide resolved
once PR #634 is ready, it can be utilized as DI hook. |
That's right, I didn't notice the core point for di hook is to create the function class and register it in di framework bean container. In this case probably framework like Quarkus and Spring cannot utilize the middleware invocation pipeline. Let me discuss with the team on this. Thanks for your reply. |
9391b77
to
462b977
Compare
462b977
to
d25d64e
Compare
@kaibocai I've updated my WIP to refactor some things and add some tests. FYI: Could create some kind of FunctionFactory interface, just thought it would be less a burdon on MSFT and anybody writing an extension if they didn't have to include azure-functions-java-worker dependency. |
d25d64e
to
78ab223
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Hi @patriot1burke , just want to confirm that for Quarkus to work for azure function, is it a must that function class need to be created/registered by/to Quarkus bean container , can we just initialize the framework without the next line. Thank you. BTW maybe update the commit message as well. |
Yes, its required that Quarkus create the function class. Please see the example of what the integration would look like. I imagine that Spring would want the same thing. |
take this forward rename services file
78ab223
to
4fdafe4
Compare
Hi @patriot1burke , sorry for late reply (not seeing the notification of your comment), this PR #634 didn't build cause we haven't update necessary dependencies, which is also required for this PR. Will make the updates asap. Please feel free to branch off this new PR #641, which has a clear implementation. azure-functions-java-worker/pom.xml Line 69 in ac6cb81
Please let me know if anything else you need from myside. Appreciate your contribution! Thank you very much. |
@kaibocai Saw that your 'middleware" changes made it into master! :) Has this version of the java worker been released? When will it be available to use with Azure Functions live? TY!! |
@kaibocai will the ability to change the target function class instance be added to the MiddlewareContext interface as we discussed in previous exchanges? I'm would still be unable to integrate Quarkus with the current java worker in main. |
Hi @patriot1burke , thanks very much for reaching out. After detailed discussion with whole team, we moved out DI hook from middleware feature, we are currently actively working on PR DI hook feature and PR spi library, it will provide an interface that you can use to initialize the framework. We are looking forward to release this feature in next two weeks. Thanks very much for your help and contribution. BTW: we are using dev branch as latest development branch. Thank you. |
Thank you @kaibocai I will update the Quarkus integration code once this gets merged to dev. Ty for help. |
close this PR as PR #667 is merged |
This is a work in progress. Will add tests if there's interest in this. This simple patch provides a hook so that DI frameworks can instantiate function class instances.
In my PoC, I was able to hook in Quarkus Arc CDI implementation using this WIP quarkus azure extension so that an Azure function class could be a CDI bean.
JavaMethodExecutor looks for a text file META-INF/service/com.microsoft.azure.functions.FunctionFactory in function app. If exists, reads that file to find a FQN classname. Does reflection to find a method named "newInstande(Class)". If this static method exists, it will use that to allocate a function class instance instead of class.newInstance().