Skip to content

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

Closed
wants to merge 1 commit into from

Conversation

patriot1burke
Copy link

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().

@msftclas
Copy link

msftclas commented Jul 26, 2019

CLA assistant check
All CLA requirements met.

@kaibocai
Copy link
Member

kaibocai commented Aug 8, 2022

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.

@patriot1burke
Copy link
Author

@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.

@patriot1burke
Copy link
Author

@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!

kaibocai
kaibocai previously approved these changes Aug 19, 2022
Copy link
Member

@kaibocai kaibocai left a 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.

@kaibocai
Copy link
Member

kaibocai commented Aug 24, 2022

once PR #634 is ready, it can be utilized as DI hook.
@patriot1burke please review above PR and give any feedback you would have. (Feedback from expert like you helps us a lot.) Thank you.

@patriot1burke
Copy link
Author

@kaibocai #634 is still allocating the function class in JavaMethodExecutorImpl. Would want to delegate function class instantiation to another framework (Quarkus or Spring).

@kaibocai
Copy link
Member

kaibocai commented Aug 24, 2022

@kaibocai #634 is still allocating the function class in JavaMethodExecutorImpl. Would want to delegate function class instantiation to another framework (Quarkus or Spring).

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.

@patriot1burke
Copy link
Author

@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.

@kaibocai
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kaibocai
Copy link
Member

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.

@patriot1burke
Copy link
Author

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
@kaibocai
Copy link
Member

kaibocai commented Sep 2, 2022

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.
The dependency PR is here which you can build it on you local for testing/implementing the DI hook Azure/azure-functions-java-additions#7 at

<groupId>com.microsoft.azure.functions</groupId>

Please let me know if anything else you need from myside. Appreciate your contribution!
Thank you very much.

@patriot1burke
Copy link
Author

@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!!

@patriot1burke
Copy link
Author

@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.

@kaibocai
Copy link
Member

@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!!

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.

@patriot1burke
Copy link
Author

@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!!

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.

@kaibocai
Copy link
Member

close this PR as PR #667 is merged

@kaibocai kaibocai closed this Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants