Skip to content

refactor java worker - add middleware support #652

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

Merged
merged 8 commits into from
Oct 5, 2022

Conversation

kaibocai
Copy link
Member

@kaibocai kaibocai commented Sep 19, 2022

Issue describing the changes in this PR

This PR include two features:

1st feature

mainly decoupled function definition (build at function load request processing) and function executor (build at invocation request process). It includes updates as below

  • The worker used to build function executor at FunctionLoadRequest, now updated the logic to build the executor at InvocationRequest.
  • Update the executor only depends on the execution context passed to it at every invocation request, make the execution logic more clear.
  • Making ExecutionContextDataSource class to public, as this class is only used internal in java worker, it should cause any issue on changing it's scope to public.
  • Update ExecutionContextDataSource to use builder pattern as it has to many arguments in old constructor.
  • Adding three fields dataStore, methodBindInfo, containingClass to ExecutionContextDataSource, since decoupled the function definition and function executor, now function executor relies on ExecutionContextDataSource to get all required objects.
  • Update MethodBindInfo, ParamBindInfo from nested class to top class as they are used by other class besides their parent class.

2nd feature

Add support for middleware.
This PR along with core library PR #11 and durable java sdk PR #87 resolve #595

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests) -- to be done.

Additional information

Additional PR information

@kaibocai
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

joint code review with Kaibo and Shreyas:

trask
trask previously approved these changes Sep 23, 2022
* middleware support logics

* refactor code

* rename middleware chain

* refactor code

* fix error

* refactor code

* minor updates

* remove used local varaible

* APIs change accordingly

* update method name

* update method name
@kaibocai
Copy link
Member Author

rebase the dev branch

@kaibocai kaibocai changed the title decouple function definition and function executor refactor java worker - add middleware support Sep 27, 2022
Copy link
Member

@shreyas-gopalakrishna shreyas-gopalakrishna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cold start results look good

@shreyas-gopalakrishna shreyas-gopalakrishna merged commit c1895b5 into dev Oct 5, 2022
@shreyas-gopalakrishna shreyas-gopalakrishna deleted the kaibocai/de-java811 branch October 5, 2022 19:55
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.

Custom middleware for function triggers
3 participants