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

Add API Key Based Authentication for Ballerina Twilio Connector #238

Merged

Conversation

SachinAkash01
Copy link
Member

@SachinAkash01 SachinAkash01 commented Nov 6, 2024

Purpose

$subject

Fixes: #6245
See Also: #7343

Checklist

  • Linked to an issue
  • Updated the specification
  • Updated the changelog
  • Added tests
  • Added examples
  • Checked native-image compatibility

README.md Outdated Show resolved Hide resolved
ballerina/Ballerina.toml Outdated Show resolved Hide resolved
ballerina/types.bal Outdated Show resolved Hide resolved
ballerina/types.bal Outdated Show resolved Hide resolved
ballerina/types.bal Outdated Show resolved Hide resolved
SachinAkash01 and others added 3 commits November 7, 2024 09:16
Co-authored-by: Ayesh Almeida <77491511+ayeshLK@users.noreply.github.com>
Co-authored-by: Ayesh Almeida <77491511+ayeshLK@users.noreply.github.com>
Co-authored-by: Ayesh Almeida <77491511+ayeshLK@users.noreply.github.com>
ballerina/tests/tests.bal Outdated Show resolved Hide resolved
gradle.properties Outdated Show resolved Hide resolved
Copy link
Contributor

@NipunaRanasinghe NipunaRanasinghe left a comment

Choose a reason for hiding this comment

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

@SachinAkash01 we also need to update the docs and examples with the new changes

@SachinAkash01
Copy link
Member Author

@SachinAkash01 we also need to update the docs and examples with the new changes

Added examples with new changes.

@SachinAkash01 SachinAkash01 marked this pull request as ready for review November 7, 2024 11:28
@SachinAkash01 SachinAkash01 requested a review from ayeshLK November 7, 2024 11:28
Copy link
Member

@ayeshLK ayeshLK left a comment

Choose a reason for hiding this comment

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

Shall we build the package using gradle so that relevant version information would be updated automatically ?

@ayeshLK
Copy link
Member

ayeshLK commented Nov 7, 2024

Along with this change we have to update our documentation Module.md, Package.md and Readme.md by updating the Setup guide and Quickstart sections with API Key-secret based authentication model. I think it should be in a different PR.

Copy link
Member

@ayeshLK ayeshLK left a comment

Choose a reason for hiding this comment

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

We need to update the wrapper-sanitations.md [1] with the changes we did for the ConnectionConfig. Shall we do that as well ?

[1] - https://github.com/ballerina-platform/module-ballerinax-twilio/blob/master/docs/spec/wrapper-sanitization.md

@SachinAkash01
Copy link
Member Author

SachinAkash01 commented Nov 7, 2024

Along with this change we have to update our documentation Module.md, Package.md and Readme.md by updating the Setup guide and Quickstart sections with API Key-secret based authentication model. I think it should be in a different PR.

Yes, I'm working on it and will send a separate PR for all the changes related to documentation.

We need to update the wrapper-sanitations.md [1] with the changes we did for the ConnectionConfig. Shall we do that as well ?

[1] - https://github.com/ballerina-platform/module-ballerinax-twilio/blob/master/docs/spec/wrapper-sanitization.md

Since this is also related to documentation activity, I thought of adding it to the same PR I have mentioned above.. @ayeshLK

ballerina/types.bal Outdated Show resolved Hide resolved
SachinAkash01 and others added 4 commits November 8, 2024 09:59
Co-authored-by: Ayesh Almeida <77491511+ayeshLK@users.noreply.github.com>
Co-authored-by: Ayesh Almeida <77491511+ayeshLK@users.noreply.github.com>
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

Attention: Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.

Project coverage is 26.53%. Comparing base (da92d96) to head (8f67ce9).
Report is 24 commits behind head on master.

Files with missing lines Patch % Lines
ballerina/client.bal 89.28% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
+ Coverage   22.41%   26.53%   +4.12%     
==========================================
  Files           8        5       -3     
  Lines        2838     2355     -483     
  Branches     1067     1077      +10     
==========================================
- Hits          636      625      -11     
+ Misses       2202     1730     -472     

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

@NipunaRanasinghe
Copy link
Contributor

NipunaRanasinghe commented Nov 8, 2024

@SachinAkash01 Please use the "Add suggestion to batch" feature (attached below) instead of applying each suggestion individually. This will keep the commit history cleaner and reduce the need for squashing.
Screenshot 2024-11-08 at 11 32 58

Copy link
Member

@ayeshLK ayeshLK left a comment

Choose a reason for hiding this comment

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

LGTM

@NipunaRanasinghe NipunaRanasinghe merged commit 1bd2fe1 into ballerina-platform:master Nov 9, 2024
4 checks passed
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.

Add support for API key for twilio connector[4.x.x] authentication
4 participants