chore: Dummy to run CI on external contributor's PR : add support for Google Cloud Storage in S3 plugin#34010
chore: Dummy to run CI on external contributor's PR : add support for Google Cloud Storage in S3 plugin#34010NilanshBansal wants to merge 12 commits intoreleasefrom
Conversation
…b.com:AnnaHariprasad5123/appsmith into chore/issue-6877/dummy-external-33938
github-actions
bot
added
the
skip-changelog
WalkthroughThe recent updates introduce support for Google Cloud Storage in the Amazon S3 plugin. Key changes include adding constants and logic to handle Google Cloud Storage endpoints in the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant Application
participant DatasourceUtils
participant S3ClientBuilder
User->>Application: Configure Google Cloud Storage
Application->>DatasourceUtils: Call getS3ClientBuilder
DatasourceUtils->>S3ClientBuilder: Handle Google Cloud Storage endpoint
S3ClientBuilder-->>DatasourceUtils: Return configured builder
DatasourceUtils-->>Application: Return S3 client
Application-->>User: Confirm configuration
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java (1)
102-102: Refactor imports to avoid wildcard imports.Using wildcard imports can lead to namespace pollution and make it unclear which classes are being used. Consider importing only the necessary classes explicitly.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/constants/S3PluginConstants.java (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/exceptions/S3ErrorMessages.java (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java (6 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/resources/form.json (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java (3 hunks)
Files skipped from review due to trivial changes (1)
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java
Additional comments not posted (11)
app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/constants/S3PluginConstants.java (3)
8-8: LGTM! Ensure that the index3aligns with the actual position in the properties array.
17-17: LGTM! Ensure that this constant is used consistently across the plugin for Google Cloud Storage.
18-18: LGTM! Please verify the usage ofAUTOacross the plugin to ensure it aligns with its intended purpose.#!/bin/bash # Description: Verify the usage of the `AUTO` constant across the plugin. # Test: Search for the constant usage. Expect: Relevant and consistent usage. rg --type java $'AUTO'app/server/appsmith-plugins/amazons3Plugin/src/main/resources/form.json (2)
46-49: LGTM! The new dropdown option for Google Cloud Storage is correctly added.
115-125: LGTM! The conditional visibility of the "Default Bucket" field based on the storage option is correctly implemented.app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/exceptions/S3ErrorMessages.java (2)
21-22: LGTM! The error message for non-existent buckets in Google Cloud Storage is clear and specific.
133-136: LGTM! The error message for missing default bucket parameter in Google Cloud Storage is clear and specific.app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java (2)
68-68: LGTM! The addition ofGOOGLE_CLOUD_STORAGEto theS3ServiceProviderenum is correctly implemented.
157-163: LGTM! The updated logic to handle Google Cloud Storage endpoints ingetS3ClientBuilderis correctly implemented.#!/bin/bash # Description: Verify the usage of the `AUTO` constant in the context of Google Cloud Storage. # Test: Search for the constant usage in the method. Expect: Relevant and consistent usage. rg --type java $'AUTO' --context 5app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java (2)
951-961: Validate the presence of the default bucket for Google Cloud Storage.The validation logic correctly checks if the default bucket is specified when the service provider is Google Cloud Storage. This is crucial for ensuring that the datasource configuration is complete before attempting operations.
1015-1041: Implement a separate testing method for Google Cloud Storage.The new method
testGoogleCloudStorageis a good addition for modularizing the testing logic specific to Google Cloud Storage. This helps in isolating the testing logic per provider, making the code cleaner and easier to manage.
...erver/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java
Show resolved
Hide resolved
…b.com:AnnaHariprasad5123/appsmith into chore/issue-6877/dummy-external-33938
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java (3 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/constants/S3PluginConstants.java (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/exceptions/S3ErrorMessages.java (2 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java (5 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (5)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/constants/S3PluginConstants.java
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/exceptions/S3ErrorMessages.java
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java
…b.com:AnnaHariprasad5123/appsmith into chore/issue-6877/dummy-external-33938
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java (6 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java
…b.com:AnnaHariprasad5123/appsmith into chore/issue-6877/dummy-external-33938
|
Closing this PR in lieu of #34075 |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java (6 hunks)
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/utils/DatasourceUtils.java
- app/server/appsmith-plugins/amazons3Plugin/src/test/java/com/external/plugins/AmazonS3PluginTest.java
2 participants
Description
The original PR is #33938
CI cannot be run on PR which is from a fork other than appsmith.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9400512558
Commit: 0552330
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests