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

Implementation of Dapr Building Block: Bindings #7970

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

SoTrx
Copy link
Contributor

@SoTrx SoTrx commented Sep 27, 2024

Description

Hey,
This PR introduces support for Dapr Bindings

Type of change

  • This pull request adds or changes features of Radius and has an approved issue (issue link required).

Fixes: #7960

@SoTrx SoTrx requested review from a team as code owners September 27, 2024 13:10
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 83 lines in your changes missing coverage. Please review.

Project coverage is 59.48%. Comparing base (5549e1e) to head (6690650).

Files with missing lines Patch % Lines
pkg/daprrp/processors/bindings/processor.go 42.16% 42 Missing and 6 partials ⚠️
pkg/daprrp/datamodel/daprbinding.go 0.00% 13 Missing ⚠️
pkg/daprrp/setup/setup.go 47.05% 8 Missing and 1 partial ⚠️
...g/corerp/backend/deployment/deploymentprocessor.go 0.00% 6 Missing ⚠️
.../daprrp/api/v20231001preview/binding_conversion.go 97.16% 2 Missing and 1 partial ⚠️
...kg/daprrp/datamodel/converter/binding_converter.go 86.36% 2 Missing and 1 partial ⚠️
pkg/rp/portableresources/portableresources.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7970      +/-   ##
==========================================
+ Coverage   59.46%   59.48%   +0.02%     
==========================================
  Files         577      581       +4     
  Lines       38644    38893     +249     
==========================================
+ Hits        22978    23135     +157     
- Misses      13998    14078      +80     
- Partials     1668     1680      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lakshmimsft
Copy link
Contributor

lakshmimsft commented Oct 1, 2024

@SoTrx, Thank you again for the amazing contributions!
Can I request that you split this into multiple PRs, say one for Dapr Bindings and another for Dapr Components Scoping. It's currently pretty large to be able to go through and review effectively.
Also, pls update the issue #7960 with proposed schema changes as you had updated for Configuration Store in #7889.
Thank you!

@SoTrx
Copy link
Contributor Author

SoTrx commented Oct 5, 2024

Sure !

@SoTrx SoTrx force-pushed the feat_dapr-bindings branch 3 times, most recently from a7fea59 to 6757856 Compare October 5, 2024 13:35
@SoTrx SoTrx changed the title Implementation of Dapr Building Block: Bindings and Components Scoping Implementation of Dapr Building Block: Bindings Oct 5, 2024
@SoTrx
Copy link
Contributor Author

SoTrx commented Oct 5, 2024

I've removed the scopes implementations from this one and made the proposed schema changes for the Bindings Building Block.

I just saw the new PR checklist—should I also create a PR in the design notes repo?

@lakshmimsft
Copy link
Contributor

I've removed the scopes implementations from this one and made the proposed schema changes for the Bindings Building Block.

I just saw the new PR checklist—should I also create a PR in the design notes repo?

Thank you for the update and for providing detailed information in the issue: #7960. ccing @rynowak, @kachawla, and @willtsai for input as well.

@rynowak
Copy link
Contributor

rynowak commented Oct 9, 2024

I've removed the scopes implementations from this one and made the proposed schema changes for the Bindings Building Block.

I just saw the new PR checklist—should I also create a PR in the design notes repo?

If you have time, that would be awesome. We like to see a design document for API changes. Adding a new resource type is definitely design-doc worthy.

There are lots of examples, feel free to ping any of us on discord if you want pointers or feedback.

@lakshmimsft
Copy link
Contributor

Hello @SoTrx, let us know whenever this PR is updated per the design document discussions, and we'll make progress with the PR review for it. Thanks again!

namespace Applications.Dapr;

@doc("Dapr binding portable resource")
model DaprBindingResource
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: Should I rename this from "DaprBindings" to "Binding" as per the design notes? Some Dapr components use the "Dapr" prefix, such as statestore, while others do not, such as secretstore.

@SoTrx
Copy link
Contributor Author

SoTrx commented Dec 5, 2024

Hello @SoTrx, let us know whenever this PR is updated per the design document discussions, and we'll make progress with the PR review for it. Thanks again!

Should be good to go !

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.

Implementation of Dapr Building Block: Bindings
3 participants