Skip to content
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

[CDAP-21096] Split Appfabric into stateless service and stateful processor #15773

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

vsethi09
Copy link
Contributor

@vsethi09 vsethi09 commented Dec 16, 2024

Context

Make appfabric service stateless by splitting it into appfabric service to server HTTP requests and appfabric processor to run subscriber services and process message.

Change Description

  • Added new main class AppFabricProcessorServiceMain for appfabric processor.
  • Added AppFabricProcessorService which is used by AppFabricProcessorServiceMain.
  • Removed messaging subscriber services from AppFabricServer and moved them to AppFabricProcessorService.
  • HTTP handlers are hosted only in AppFabricServer and AppFabricProcessorService does not have any HTTP handlers.
  • Updated test classes to run Appfabric Processor along with Appfabric Server wherever needed.

Verification

  • Unit Tests:
    • Updated test classes to run Appfabric Processor along with Appfabric Server wherever needed.
    • Added UT for AppFabricProcessorServiceTest to test Start and Stop (similar to AppFabricServerTest).
    • Note: UT cannot be added for AppFabricProcessorServiceMain as it does not host and HTTP handlers and runs only subscriber services.
  • CDAP sandbox: Tested basic flows.
  • Deployed locally built image to a k8s cluster and verified by deployed CRD and CR. ([CDAP-21096] Split Appfabric into stateless service and stateful processor cdap-operator#123)

@vsethi09 vsethi09 added the build Triggers github actions build label Dec 16, 2024
@vsethi09 vsethi09 force-pushed the feature/CDAP-21096_split_appfabric_service_processor branch 7 times, most recently from 6874823 to 75ec19d Compare December 19, 2024 09:43
@vsethi09 vsethi09 marked this pull request as ready for review December 19, 2024 09:43
@vsethi09 vsethi09 changed the title [WIP][CDAP-21096] Split Appfabric into stateless service and stateful processor [CDAP-21096] Split Appfabric into stateless service and stateful processor Dec 19, 2024
@vsethi09 vsethi09 force-pushed the feature/CDAP-21096_split_appfabric_service_processor branch from 75ec19d to b3e3e17 Compare December 19, 2024 10:11
Comment on lines 102 to 103
CredentialProviderService credentialProviderService,
NamespaceCredentialProviderService namespaceCredentialProviderService,
Copy link
Member

@itsankit-google itsankit-google Dec 19, 2024

Choose a reason for hiding this comment

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

why do we need CredentialProviderService in processor?

Copy link
Member

Choose a reason for hiding this comment

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

How will it work in this case if the processor service does not host any handlers but GcpWorkloadIdentityHttpHandler runs in AppfabricService?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed. Removed.

@@ -0,0 +1,216 @@
/*
* Copyright © 2014-2020 Cask Data, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

nit: 2024

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@vsethi09 vsethi09 force-pushed the feature/CDAP-21096_split_appfabric_service_processor branch from b3e3e17 to f88bc89 Compare December 23, 2024 09:38
@vsethi09 vsethi09 force-pushed the feature/CDAP-21096_split_appfabric_service_processor branch from f88bc89 to ce8bda2 Compare December 23, 2024 11:06
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
42.2% Coverage on New Code (required ≥ 80%)
10.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Triggers github actions build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants