-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[Components] ecologi #13287 #15577
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
[Components] ecologi #13287 #15577
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
Warning Rate limit exceeded@lcaresia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis pull request removes legacy files and introduces new modules that integrate with the Ecologi API. The changes include deleting a Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Caller
participant A as Action Module (buy-trees/offsets)
participant EA as Ecologi App Module
participant R as HTTP Request Handler
U->>A: Invoke run() with parameters
A->>EA: Call buyTrees()/buyOffsets() method
EA->>R: Trigger _makeRequest() with API details
R-->>EA: Return API response
EA-->>A: Pass response to action module
A-->>U: Return summary message and response
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/ecologi/actions/buy-trees/buy-trees.mjs (1)
30-41: Consider enhancing the success message with more details.While the implementation is correct, the success message could be more informative by including the test mode status.
Apply this diff to enhance the success message:
- $.export("$summary", `Successfully bought ${this.number} tree(s)`); + $.export("$summary", `Successfully bought ${this.number} tree(s)${this.test ? " in test mode" : ""}`);components/ecologi/actions/buy-offsets/buy-offsets.mjs (1)
30-41: Enhance the success message with purchase details.The success message could be more informative by including the number of offsets, units, and test mode status.
Apply this diff to enhance the success message:
- $.export("$summary", "Successfully bought carbon avoidance credits"); + $.export("$summary", `Successfully bought ${this.number} ${this.units.toLowerCase()} of carbon avoidance credits${this.test ? " in test mode" : ""}`);components/ecologi/ecologi.app.mjs (1)
33-35: Consider making the base URL configurable.The base URL is hardcoded. Consider making it configurable through environment variables or app configuration to support different environments (e.g., staging).
Apply this diff to make the base URL configurable:
_baseUrl() { - return "https://public.ecologi.com"; + return this.$auth.base_url || "https://public.ecologi.com"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/ecologi/.gitignore(0 hunks)components/ecologi/actions/buy-offsets/buy-offsets.mjs(1 hunks)components/ecologi/actions/buy-trees/buy-trees.mjs(1 hunks)components/ecologi/app/ecologi.app.ts(0 hunks)components/ecologi/common/constants.mjs(1 hunks)components/ecologi/ecologi.app.mjs(1 hunks)components/ecologi/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- components/ecologi/.gitignore
- components/ecologi/app/ecologi.app.ts
✅ Files skipped from review due to trivial changes (1)
- components/ecologi/common/constants.mjs
🧰 Additional context used
🪛 GitHub Actions: Pull Request Checks
components/ecologi/package.json
[error] 1-1: Cannot install with 'frozen-lockfile' because pnpm-lock.yaml is not up to date with package.json.
🪛 GitHub Actions: Components Checks
components/ecologi/package.json
[warning] 1-1: Unsupported engine: wanted: {"node":"20.9.0"} (current: {"node":"v18.20.6","pnpm":"9.14.2"})
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pnpm publish
🔇 Additional comments (1)
components/ecologi/actions/buy-trees/buy-trees.mjs (1)
3-9: LGTM! Well-structured module with clear metadata.The module is well-organized with appropriate key, name, description (including documentation link), version, and type. The documentation link helps users understand the API endpoint.
| async _makeRequest(opts = {}) { | ||
| const { | ||
| $ = this, | ||
| path, | ||
| headers, | ||
| ...otherOpts | ||
| } = opts; | ||
| return axios($, { | ||
| ...otherOpts, | ||
| url: this._baseUrl() + path, | ||
| headers: { | ||
| "Authorization": `Bearer ${this.$auth.api_key}`, | ||
| "Accept": "application/json", | ||
| ...headers, | ||
| }, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling to the request helper.
The _makeRequest method should include error handling to provide meaningful error messages.
Apply this diff to add error handling:
async _makeRequest(opts = {}) {
const {
$ = this,
path,
headers,
...otherOpts
} = opts;
- return axios($, {
- ...otherOpts,
- url: this._baseUrl() + path,
- headers: {
- "Authorization": `Bearer ${this.$auth.api_key}`,
- "Accept": "application/json",
- ...headers,
- },
- });
+ try {
+ return await axios($, {
+ ...otherOpts,
+ url: this._baseUrl() + path,
+ headers: {
+ "Authorization": `Bearer ${this.$auth.api_key}`,
+ "Accept": "application/json",
+ ...headers,
+ },
+ });
+ } catch (err) {
+ throw new Error(`Ecologi API request failed: ${err.message}`);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async _makeRequest(opts = {}) { | |
| const { | |
| $ = this, | |
| path, | |
| headers, | |
| ...otherOpts | |
| } = opts; | |
| return axios($, { | |
| ...otherOpts, | |
| url: this._baseUrl() + path, | |
| headers: { | |
| "Authorization": `Bearer ${this.$auth.api_key}`, | |
| "Accept": "application/json", | |
| ...headers, | |
| }, | |
| }); | |
| }, | |
| async _makeRequest(opts = {}) { | |
| const { | |
| $ = this, | |
| path, | |
| headers, | |
| ...otherOpts | |
| } = opts; | |
| try { | |
| return await axios($, { | |
| ...otherOpts, | |
| url: this._baseUrl() + path, | |
| headers: { | |
| "Authorization": `Bearer ${this.$auth.api_key}`, | |
| "Accept": "application/json", | |
| ...headers, | |
| }, | |
| }); | |
| } catch (err) { | |
| throw new Error(`Ecologi API request failed: ${err.message}`); | |
| } | |
| } |
| { | ||
| "name": "@pipedream/ecologi", | ||
| "version": "0.0.2", | ||
| "version": "0.1.0", | ||
| "description": "Pipedream Ecologi Components", | ||
| "main": "dist/app/ecologi.app.mjs", | ||
| "keywords": [ | ||
| "pipedream", | ||
| "ecologi" | ||
| ], | ||
| "files": ["dist"], | ||
| "homepage": "https://pipedream.com/apps/ecologi", | ||
| "author": "Pipedream <support@pipedream.com> (https://pipedream.com/)", | ||
| "publishConfig": { | ||
| "access": "public" | ||
| }, | ||
| "dependencies": { | ||
| "@pipedream/platform": "^3.0.3" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update pnpm-lock.yaml and specify Node.js version.
The pipeline is failing due to:
- Out-of-date lock file
- Node.js version mismatch (wanted: 20.9.0, current: 18.20.6)
Apply this diff to specify the Node.js version:
"dependencies": {
"@pipedream/platform": "^3.0.3"
- }
+ },
+ "engines": {
+ "node": "20.9.0"
+ }
}Then run the following commands to update the lock file:
pnpm install🧰 Tools
🪛 GitHub Actions: Pull Request Checks
[error] 1-1: Cannot install with 'frozen-lockfile' because pnpm-lock.yaml is not up to date with package.json.
🪛 GitHub Actions: Components Checks
[warning] 1-1: Unsupported engine: wanted: {"node":"20.9.0"} (current: {"node":"v18.20.6","pnpm":"9.14.2"})
jcortes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lcaresia lgtm! Ready for QA!
WHY
Summary by CodeRabbit
New Features
Chores