-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Nasdaq new components #18158
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
Nasdaq new components #18158
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds Nasdaq Data Link app integration with axios-based requests, pagination, and prop definitions. Introduces three actions: get table metadata, get table data (with pagination and optional filters/columns), and export table (bulk download). Adds a JSON parsing utility and updates package metadata and dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Get Table Data Action
participant App as Nasdaq DL App
participant API as Nasdaq API
User->>Action: Run with publisher/table, columns, filters
Action->>App: paginate(fn=tableData, args)
loop Up to maxRequests (default 3)
App->>API: GET /datatables/{publisher}/{table}.json<br/>params: filters, qopts.columns, per_page=100, cursor
API-->>App: rows[], meta.next_cursor_id
App-->>App: Accumulate rows
end
App-->>Action: Combined rows[]
Action-->>User: Summary with record count + data
sequenceDiagram
autonumber
actor User
participant Action as Export Table Action
participant App as Nasdaq DL App
participant API as Nasdaq API
User->>Action: Run with publisher/table, columns, filters
Action->>App: tableData(qopts.export=true, filters, qopts.columns)
App->>API: GET /datatables/{publisher}/{table}.json
API-->>App: datatable_bulk_download { status, link? }
App-->>Action: Response
Action-->>User: Summary based on status (fresh/creating/regenerating)
sequenceDiagram
autonumber
actor User
participant Action as Get Table Metadata Action
participant App as Nasdaq DL App
participant API as Nasdaq API
User->>Action: Run with publisher/table
Action->>App: tableMetadata(publisher, table)
App->>API: GET /datatables/{publisher}/{table}/metadata.json
API-->>App: Metadata (columns, types, etc.)
App-->>Action: Response
Action-->>User: Success summary + metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 6
🧹 Nitpick comments (7)
components/nasdaq_data_link_time_series_and_table_data_/package.json (1)
15-17
: Dependency looks appropriate; consider pinning strategy and adding metadata.
- Adding @pipedream/platform is expected for modern components.
- Consider adding repository and license fields for better package hygiene.
- If reproducibility is a concern, consider pinning to a specific minor (e.g., "~3.1.0") or keeping "^" but verifying compatibility in CI when 3.x updates land.
"author": "Pipedream <support@pipedream.com> (https://pipedream.com/)", + "repository": { + "type": "git", + "url": "https://github.com/PipedreamHQ/pipedream" + }, + "license": "MIT", "publishConfig": { "access": "public" }, "dependencies": { "@pipedream/platform": "^3.1.0" }components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-metadata/get-table-metadata.mjs (1)
31-39
: Optional: add targeted error handling for 404 to improve UX.
Letting errors bubble is fine, but a concise summary on 404 helps users correct publisher/table typos quickly.- const response = await app.tableMetadata({ - $, - publisher, - table, - }); + let response; + try { + response = await app.tableMetadata({ + $, + publisher, + table, + }); + } catch (err) { + if (err?.response?.status === 404) { + $.export("$summary", `Table \`${publisher}/${table}\` was not found`); + } + throw err; + }components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs (1)
59-76
: Consider exposing perPage/maxPages as optional props for flexibility.
This would let users tune throughput vs. rate limits without editing the shared app.props: { app, + perPage: { + type: "integer", + label: "Rows per Page", + description: "Optional. Overrides the default rows per page used by the app (qopts.per_page).", + optional: true, + default: undefined, + }, + maxPages: { + type: "integer", + label: "Max Pages", + description: "Optional. Limit the number of pages to fetch.", + optional: true, + default: undefined, + },And pass through in paginate args if defined.
components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs (1)
79-87
: Avoid asserting link TTL; handle failure status explicitly.
Unless guaranteed by docs, avoid stating “valid for 30 minutes.” Also cover a possible “failed” status.- if (status === "fresh" && link) { - $.export("$summary", `Table ${publisher}/${table} is ready for download. The download link is valid for 30 minutes.`); - - } else if (status === "creating" || status === "regenerating") { + if (status === "fresh" && link) { + $.export("$summary", `Table ${publisher}/${table} is ready for download.`); + } else if (status === "creating" || status === "regenerating") { $.export("$summary", `Export job for table ${publisher}/${table} is ${status}. Please retry in a few moments to get the download link.`); - } else { + } else if (status === "failed") { + $.export("$summary", `Export job for table ${publisher}/${table} failed. Please adjust filters or retry later.`); + } else { $.export("$summary", `Export initiated for table ${publisher}/${table}`); }If you prefer to keep the TTL claim, please confirm the current validity window in the Nasdaq docs and restore it with a citation inline in the code comment.
components/nasdaq_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs (3)
22-34
: Harden dynamic column options and improve UXWrap the metadata call in a try/catch so the UI doesn’t break if the API rejects/ratelimits or the user mistypes the codes. Also consider returning { label, value } with the column type for clarity.
Apply this diff:
async options({ publisher, table, }) { if (!publisher || !table) { return []; } - const { datatable: { columns } } = await this.tableMetadata({ - publisher, - table, - }); - return columns.map(({ name }) => name); + try { + const { datatable: { columns } = {} } = await this.tableMetadata({ + publisher, + table, + }); + return (columns || []).map(({ name, type }) => ({ + label: `${name}${type ? ` (${type})` : ""}`, + value: name, + })); + } catch (err) { + // Avoid failing the prop UI when publisher/table are invalid or metadata is restricted + console.error("Failed to load column options", err?.response?.data ?? err?.message); + return []; + } },
73-105
: Paginate defaults are conservative; consider configurability and safeguardsmaxRequests defaults to 3 (≈300 rows at 100/page), which may be too shallow for many tables. Expose page limit (and possibly total row limit) via args, and surface minimal metadata (e.g., last cursor) for debugging. Optionally short-circuit if args.params already set a smaller qopts.per_page.
Example tweak within current structure:
- async paginate({ - fn, args = {}, maxRequests = 3, - } = {}) { + async paginate({ + fn, args = {}, maxRequests = 10, maxRows, + } = {}) { let allData = []; let cursorId = null; let requestCount = 0; let hasMorePages = true; while (hasMorePages && requestCount < maxRequests) { const response = await fn({ ...args, params: { ...args.params, - "qopts.per_page": 100, + "qopts.per_page": args?.params?.["qopts.per_page"] ?? 100, ...(cursorId ? { "qopts.cursor_id": cursorId, } : undefined ), }, }); const pageData = response?.datatable?.data || []; - allData = allData.concat(pageData); + allData = maxRows + ? allData.concat(pageData).slice(0, maxRows) + : allData.concat(pageData); cursorId = response?.meta?.next_cursor_id; - hasMorePages = !!cursorId; + hasMorePages = !!cursorId && (!maxRows || allData.length < maxRows); requestCount++; } return allData; },If you want, I can open a follow-up PR to add an optional maxRows prop to the action that calls paginate.
40-46
: Confirm header-based auth is reliable across all endpointsNasdaq Data Link typically supports the X-Api-Token header, but some examples prefer the api_key query param. Keeping header-only is cleaner, but if you hit auth oddities, consider supporting both (while keeping debug off to avoid leaking secrets).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs
(1 hunks)components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs
(1 hunks)components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-metadata/get-table-metadata.mjs
(1 hunks)components/nasdaq_data_link_time_series_and_table_data_/common/utils.mjs
(1 hunks)components/nasdaq_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs
(1 hunks)components/nasdaq_data_link_time_series_and_table_data_/package.json
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/nasdaq_data_link_time_series_and_table_data_/package.json
🧬 Code graph analysis (3)
components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs (2)
components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs (1)
response
(59-76)components/nasdaq_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs (1)
response
(82-94)
components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs (2)
components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs (1)
response
(59-74)components/nasdaq_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs (1)
response
(82-94)
components/nasdaq_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs (4)
components/keboola/actions/update-table-name/update-table-name.mjs (1)
table
(35-37)components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs (1)
response
(59-74)components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs (1)
response
(59-76)components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-metadata/get-table-metadata.mjs (1)
response
(31-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (6)
components/nasdaq_data_link_time_series_and_table_data_/package.json (2)
3-3
: Version bump aligns with new features — good release discipline.
Incrementing to 0.1.0 is appropriate given the addition of multiple new actions and utilities.
1-18
: Confirmed: no built-in Node modules added as dependencies.
This aligns with prior guidance for Pipedream components (no fs, path, etc., in dependencies). Carry on.components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-metadata/get-table-metadata.mjs (1)
37-38
: LGTM: clear, user-friendly summary on success.
The success summary concisely reflects the action outcome.components/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs (1)
78-79
: Nice UX touch on the summary.
Returning the count is helpful for users composing workflows.components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs (1)
59-75
: LGTM overall: parameters align with Nasdaq bulk export expectations.
Using qopts.export and optional qopts.columns matches the Tables API semantics.components/nasdaq_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs (1)
1-1
: LGTM: Correct Pipedream axios importUsing axios from "@pipedream/platform" is the right choice here.
components/nasdaq_data_link_time_series_and_table_data_/actions/export-table/export-table.mjs
Show resolved
Hide resolved
...nents/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs
Outdated
Show resolved
Hide resolved
...nents/nasdaq_data_link_time_series_and_table_data_/actions/get-table-data/get-table-data.mjs
Show resolved
Hide resolved
components/nasdaq_data_link_time_series_and_table_data_/common/utils.mjs
Show resolved
Hide resolved
...q_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs
Outdated
Show resolved
Hide resolved
...q_data_link_time_series_and_table_data_/nasdaq_data_link_time_series_and_table_data_.app.mjs
Show resolved
Hide resolved
51052f7
to
04d1c30
Compare
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 @jcortes, LGTM! Ready for QA!
WHY
Resolves #17846
Summary by CodeRabbit
New Features
Chores