-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - polygon #15558
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
New Components - polygon #15558
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughMultiple new modules and enhancements have been added across the Polygon component. New action modules enable retrieval of company financials, historical prices, and stock prices via the Polygon API. Common utilities and constants have been introduced, and the main app component now includes additional properties and methods for API requests and pagination. Furthermore, new sources for market news, stock price summaries, and stock trade events—along with corresponding test events—have been implemented. A new package manifest now defines the component’s metadata and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as Polygon App
participant API as Polygon API
User->>App: Call getCurrentPrice(stockTicker, date)
App->>API: _makeRequest(opts)
API-->>App: Return stock price data
App-->>User: Return data with summary message
sequenceDiagram
participant Scheduler
participant Base as Base Module
participant DB as Database
participant API as Data API
Scheduler->>Base: Trigger emitEvent/run method
Base->>DB: _getLastDate()
Base->>API: Fetch paginated data (paginate)
API-->>Base: Return results with next page info
Base->>DB: _setLastDate() if new items found
Base-->>Scheduler: Emit event for each new item
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
components/bitbucket_data_center/bitbucket_data_center.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/brillium/brillium.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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 (
|
Sources - New Market Trade - New Market News - New Stock Price Summary Actions - Get Stock Price - Get Historical Prices - Get Company
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 (12)
components/polygon/polygon.app.mjs (3)
7-44: Consider adding a default value foradjusted.
Since "By default, results are adjusted" per the description, you may wish to explicitly setdefault: truefor theadjustedprop to prevent future ambiguity and ensure consistent behavior if the user doesn't select a value.
46-62: Add explicit error handling to_makeRequest.
Returning raw responses from APIs can obscure root causes when requests fail. Consider wrapping your axios call in a try/catch block to handle and log errors more gracefully.
113-141: Prevent potential infinite loop in pagination.
Consider implementing an iteration boundary or a timeout ifparseDataFnor the API incorrectly returns anextPagefor all pages. This ensures the loop won't run indefinitely in the event of unexpected responses.components/polygon/common/utils.mjs (1)
1-7: Handle invalid URL scenarios gracefully.
IfnextUrlis sometimes malformed, callingnew URL(nextUrl)may throw an error. Checking for valid URLs beforehand or wrapping it in a try/catch could help prevent unexpected runtime failures.components/polygon/sources/new-market-news/new-market-news.mjs (1)
24-27: Consider adjusting the limit parameter.Setting a high limit of 1000 items per request might impact performance and memory usage.
Consider reducing the limit and relying on pagination:
- "limit": 1000, + "limit": 100,components/polygon/sources/new-stock-trade/new-stock-trade.mjs (1)
33-35: Add price formatting.The price in the summary should be formatted for better readability.
- return `New trade for ${item.sym} at $${item.p}`; + return `New trade for ${item.sym} at $${item.p.toFixed(2)}`;components/polygon/sources/new-stock-price-summary/new-stock-price-summary.mjs (2)
34-35: Consider adjusting the limit parameter.Setting a high limit of 5000 items per request might impact performance and memory usage.
Consider reducing the limit and relying on pagination:
- "limit": 5000, + "limit": 100,
27-29: Enhance summary with price information.The summary could be more informative by including the actual price data.
- getSummary() { - return `Daily price summary for ${this.stockTicker}`; + getSummary(item) { + return `Daily price summary for ${this.stockTicker}: Open $${item.o.toFixed(2)}, Close $${item.c.toFixed(2)}`; },components/polygon/sources/common/base.mjs (2)
37-37: Use a more reliable date comparison method.
Date.parse()can be unreliable with certain date formats. Consider using a date parsing library likedate-fnsormoment.jsfor more robust date handling.
46-46: Consider timezone handling in timestamp fallback.The fallback to
new Date()doesn't consider timezone differences. Consider using UTC dates consistently.- const ts = Date.parse(item.published_utc || item.sip_timestamp || new Date()); + const ts = Date.parse(item.published_utc || item.sip_timestamp || new Date().toISOString());components/polygon/actions/get-company-financials/get-company-financials.mjs (2)
24-28: Improve description formatting.The filing date description uses template literals with manual line breaks. Consider using a more maintainable format.
- description: `Query by the date when the filing with financials data was filed in **YYYY-MM-DD** format. - \nBest used when querying over date ranges to find financials based on filings that happen in a time period. - \nExamples: - \nTo get financials based on filings that have happened after January 1, 2009 use the query param filing_date.gte=2009-01-01 - \nTo get financials based on filings that happened in the year 2009 use the query params filing_date.gte=2009-01-01&filing_date.lt=2010-01-01`, + description: [ + "Query by the date when the filing with financials data was filed in **YYYY-MM-DD** format.", + "Best used when querying over date ranges to find financials based on filings that happen in a time period.", + "Examples:", + "- To get financials based on filings that have happened after January 1, 2009 use the query param filing_date.gte=2009-01-01", + "- To get financials based on filings that happened in the year 2009 use the query params filing_date.gte=2009-01-01&filing_date.lt=2010-01-01" + ].join("\n"),
71-79: Simplify params object structure.The params object includes the stockTicker which could be passed directly to the API call.
const financialDetails = await this.polygon.getFinancialDetails({ $, + stockTicker: this.stockTicker, params: { - stockTicker: this.stockTicker, filingDate: this.filingDate, periodOfReportDate: this.periodOfReportDate, timeframe: this.timeframe, order: this.order, limit: this.limit, sort: this.sort, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
components/polygon/actions/get-company-financials/get-company-financials.mjs(1 hunks)components/polygon/actions/get-historical-prices/get-historical-prices.mjs(1 hunks)components/polygon/actions/get-stock-price/get-stock-price.mjs(1 hunks)components/polygon/common/constants.mjs(1 hunks)components/polygon/common/utils.mjs(1 hunks)components/polygon/package.json(1 hunks)components/polygon/polygon.app.mjs(1 hunks)components/polygon/sources/common/base.mjs(1 hunks)components/polygon/sources/new-market-news/new-market-news.mjs(1 hunks)components/polygon/sources/new-market-news/test-event.mjs(1 hunks)components/polygon/sources/new-stock-price-summary/new-stock-price-summary.mjs(1 hunks)components/polygon/sources/new-stock-price-summary/test-event.mjs(1 hunks)components/polygon/sources/new-stock-trade/new-stock-trade.mjs(1 hunks)components/polygon/sources/new-stock-trade/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- components/polygon/package.json
- components/polygon/common/constants.mjs
- components/polygon/sources/new-stock-price-summary/test-event.mjs
🔇 Additional comments (9)
components/polygon/polygon.app.mjs (2)
1-2: Imports appear correct.
These new imports from "@pipedream/platform" and the local utility module look consistent with the rest of the codebase.
63-112: Verify endpoint correctness and use ofadjustedparameter.
Multiple methods (e.g.,getCurrentPrice,getHistoricalPriceData, etc.) do not appear to passadjustedin the request params. If the API supports anadjustedoption, confirm that omitting it is intentional.components/polygon/sources/new-stock-trade/test-event.mjs (1)
1-14: Data structure for the test event looks good.
Providing realistic trade fields can help validate the expected shape of stock trade data. This aligns with typical usage in source tests.components/polygon/sources/new-market-news/new-market-news.mjs (2)
1-12: LGTM! Well-structured module definition.The module is well-organized with clear imports and proper metadata configuration. The description accurately reflects the module's purpose.
31-40: Consider handling empty results.The parseData method should handle cases where results array is empty or undefined.
Add validation to ensure robust error handling:
parseData({ results, next_url: next, }) { + if (!results || !Array.isArray(results)) { + return { + parsedData: [], + nextPage: null, + }; + } const parsedPage = parseNextPage(next); return { parsedData: results, nextPage: parsedPage, }; }components/polygon/sources/new-stock-trade/new-stock-trade.mjs (2)
30-32: Verify method name consistency.The method name
getPreviousCloseseems inconsistent with the module's purpose of emitting new trade events.Please verify if this is the correct method to use for fetching new trade data.
42-47: Consider validating results.Similar to the market news source, add validation for the results array.
parseData({ results }) { + if (!results || !Array.isArray(results)) { + return { + parsedData: [], + nextPage: null, + }; + } return { parsedData: results, nextPage: null, }; }components/polygon/sources/new-stock-price-summary/new-stock-price-summary.mjs (1)
41-50: Consider validating results.Similar to other sources, add validation for the results array.
parseData({ results, next_url: next, }) { + if (!results || !Array.isArray(results)) { + return { + parsedData: [], + nextPage: null, + }; + } const parsedPage = parseNextPage(next); return { parsedData: results, nextPage: parsedPage, }; }components/polygon/sources/new-market-news/test-event.mjs (1)
20-20: Update the test event date to a past date.The test event uses a future date (
2024-06-24T18:33:53Z). This could cause issues in tests that compare dates or check for recent events. Consider using a past date to ensure consistent test behavior.
components/polygon/actions/get-historical-prices/get-historical-prices.mjs
Show resolved
Hide resolved
components/polygon/actions/get-historical-prices/get-historical-prices.mjs
Show resolved
Hide resolved
components/polygon/actions/get-historical-prices/get-historical-prices.mjs
Outdated
Show resolved
Hide resolved
…l-prices.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
components/polygon/actions/get-company-financials/get-company-financials.mjs
Outdated
Show resolved
Hide resolved
components/polygon/actions/get-company-financials/get-company-financials.mjs
Show resolved
Hide resolved
components/polygon/actions/get-company-financials/get-company-financials.mjs
Outdated
Show resolved
Hide resolved
components/polygon/actions/get-company-financials/get-company-financials.mjs
Outdated
Show resolved
Hide resolved
components/polygon/actions/get-company-financials/get-company-financials.mjs
Outdated
Show resolved
Hide resolved
components/polygon/actions/get-historical-prices/get-historical-prices.mjs
Outdated
Show resolved
Hide resolved
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.
LGTM!
|
/approve |
Resolves #15141.
Summary by CodeRabbit