-
Notifications
You must be signed in to change notification settings - Fork 3
feat: coinpaprika pricing provider #92
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
base: dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe changes add support for a new pricing source called "coinpaprika" across multiple components of the system. Updates include modifications to environment configuration files (adding new variables such as COINPAPRIKA_API_KEY and COINPAPRIKA_API_TYPE) and extending allowed values for PRICING_SOURCE in both processing and bootstrap schemas. The configuration validation logic and type definitions have been updated to include "coinpaprika" alongside the previously supported "coingecko" and "dummy" options. A new provider, CoinPaprikaProvider, has been implemented to handle API requests, including token price retrieval, batching for large time ranges, and robust error handling. Furthermore, the provider factory now instantiates the new provider when the pricing source is set to "coinpaprika", and corresponding tests have been added to verify its behavior. ✨ Finishing Touches
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:
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
Documentation and Community
|
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: 1
🧹 Nitpick comments (3)
apps/processing/.env.example (1)
13-19
: Document new CoinPaprika environment variablesAdding
"coinpaprika"
as a validPRICING_SOURCE
and introducingCOINPAPRIKA_API_KEY
/COINPAPRIKA_API_TYPE
is logical. Consider adding instruction comments (in a README) on acquiring the API key and configuring tiers for better clarity.packages/pricing/src/providers/coinpaprika.provider.ts (2)
26-26
: Correct the mismatch in commentThis line references "CoinMarketCap" instead of "CoinPaprika". Update to avoid confusion.
- // CoinMarketCap API configuration + // CoinPaprika API configuration
177-193
: isTimestampWithinTierLimits methodChecks the age in days against the corresponding tier limit. If there's a possibility of future timestamps, consider whether an upper bound check is needed or if it’s out of scope.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/processing/.env.example
(1 hunks)apps/processing/src/config/env.ts
(3 hunks)packages/pricing/src/external.ts
(1 hunks)packages/pricing/src/factory/index.ts
(2 hunks)packages/pricing/src/interfaces/pricing.interface.ts
(1 hunks)packages/pricing/src/interfaces/pricingConfig.interface.ts
(1 hunks)packages/pricing/src/providers/coinpaprika.provider.ts
(1 hunks)packages/pricing/src/providers/index.ts
(1 hunks)packages/pricing/src/types/coinpaprika.types.ts
(1 hunks)packages/pricing/src/types/index.ts
(1 hunks)packages/pricing/src/types/pricing.types.ts
(2 hunks)packages/pricing/test/factory/pricingFactory.spec.ts
(2 hunks)packages/pricing/test/providers/coinpaprika.provider.spec.ts
(1 hunks)scripts/bootstrap/.env.example
(1 hunks)scripts/bootstrap/src/schemas/index.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
`**/*.ts`:
**/*.ts
:
packages/pricing/src/providers/index.ts
packages/pricing/src/external.ts
packages/pricing/src/types/index.ts
packages/pricing/src/interfaces/pricing.interface.ts
packages/pricing/src/types/pricing.types.ts
scripts/bootstrap/src/schemas/index.ts
packages/pricing/test/providers/coinpaprika.provider.spec.ts
packages/pricing/src/factory/index.ts
packages/pricing/test/factory/pricingFactory.spec.ts
apps/processing/src/config/env.ts
packages/pricing/src/types/coinpaprika.types.ts
packages/pricing/src/interfaces/pricingConfig.interface.ts
packages/pricing/src/providers/coinpaprika.provider.ts
`**/providers/**/*.ts`: Review provider files for the follow...
**/providers/**/*.ts
: Review provider files for the following:
- Providers should supply narrowly scoped data/resources.
- Ensure classes interacting with metadata sources (e.g., GitHub, JSON
files, IPFS) implement theIMetadataProvider
interface and follow
naming conventions (e.g.,GithubProvider
,JsonFileProvider
).- Be concise and avoid overly nitpicky feedback outside of these best practices.
packages/pricing/src/providers/index.ts
packages/pricing/test/providers/coinpaprika.provider.spec.ts
packages/pricing/src/providers/coinpaprika.provider.ts
`scripts/**/*.ts`: Ensure scripts: - Use `process.cwd()` f...
scripts/**/*.ts
: Ensure scripts:
- Use
process.cwd()
for root references.- Follow folder conventions (
infra/
for infra scripts,utilities/
for utilities).- Are organized in
package.json
withscript:infra:{name}
orscript:util:{name}
.- Be concise and avoid overly nitpicky feedback outside of these best practices.
scripts/bootstrap/src/schemas/index.ts
`**/*.spec.ts`: Review the unit test files with the followin...
**/*.spec.ts
: Review the unit test files with the following guidelines:
- Avoid using the word "should" in test descriptions.
- Ensure descriptive test names convey the intent of each test.
- Validate adherence to the Mocha/Chai/Jest test library best practices.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
packages/pricing/test/providers/coinpaprika.provider.spec.ts
packages/pricing/test/factory/pricingFactory.spec.ts
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: test / Unit tests
🔇 Additional comments (38)
packages/pricing/src/providers/index.ts (1)
4-4
: Export for new Coinpaprika provider added correctlyThis export follows the established pattern in the module, making the CoinPaprikaProvider accessible through the index file.
packages/pricing/src/types/pricing.types.ts (2)
1-1
: Import statement updated appropriatelyThe addition of the
Branded
type import from the shared package is appropriate for creating branded string types.
12-12
: Well-defined type for ISO8601 timestamp formatThe
TimestampISO8601
type is properly defined as a branded string type with a clear comment indicating the expected format. This type is appropriate for handling timestamps in the ISO8601 format that Coinpaprika API likely uses.packages/pricing/src/types/index.ts (1)
3-3
: Export for Coinpaprika types added correctlyThis export follows the established pattern in the module, making all the CoinPaprika-related types accessible through the index file.
packages/pricing/src/external.ts (2)
3-8
: CoinPaprikaProvider export added correctlyThe CoinPaprikaProvider has been properly added to the exported providers list, making it available to consumers of the package.
11-17
: CoinPaprikaPricingConfig type export added correctlyThe CoinPaprikaPricingConfig type has been properly added to the exported types list, maintaining consistency with the existing pattern for configuration types.
packages/pricing/src/interfaces/pricing.interface.ts (1)
6-6
: Addition of Coinpaprika provider type looks goodThe addition of "coinpaprika" to the PricingProvider union type is straightforward and correctly extends the existing options.
packages/pricing/src/types/coinpaprika.types.ts (3)
1-5
: Well-structured type definitionsGood use of the Branded type for creating a type-safe token identifier. This helps prevent string type confusions and improves code reliability.
7-13
: Comprehensive API response typeThe CoinPaprikaHistoricalResponse type properly defines all necessary fields for historical price data with appropriate types.
15-17
: Error handling type looks goodThe error response type is simple but sufficient for handling API errors.
packages/pricing/src/factory/index.ts (2)
5-5
: Import of new provider looks goodProperly importing the CoinPaprikaProvider class alongside existing providers.
51-63
: Factory implementation follows existing patternsThe implementation for the "coinpaprika" case correctly follows the same pattern as the existing providers:
- Checks for required dependencies
- Throws appropriate error if dependencies are missing
- Instantiates the provider with the necessary configuration
This maintains consistency in the codebase.
scripts/bootstrap/.env.example (1)
7-7
: Updated environment variable documentationGood addition of a comment clarifying all the available options for PRICING_SOURCE.
packages/pricing/test/providers/coinpaprika.provider.spec.ts (1)
1-208
: Well-structured test suite with descriptive test casesThe test suite for CoinPaprikaProvider is comprehensive and well-organized. It covers essential test cases including successful price retrieval, error handling scenarios, and edge cases.
I appreciate that:
- Test descriptions are clear and don't use "should" (following testing best practices)
- The test suite follows the AAA (Arrange-Act-Assert) pattern
- Edge cases like empty responses and invalid inputs are covered
- Error handling tests for different status codes (429, 401) are included
scripts/bootstrap/src/schemas/index.ts (4)
23-23
: Correct addition of "coinpaprika" to PRICING_SOURCE enumThe PRICING_SOURCE enum has been properly extended to include the new "coinpaprika" option.
37-43
: Well-defined schema for coinpaprika pricing configurationThe schema follows the same pattern as other pricing providers. The API type enumeration covers the available tiers appropriately ("free", "starter", "pro", "business", "enterprise").
46-50
: Correctly updated discriminatedUnion to include the new pricing schemaThe addition of coinpaprikaPricingSchema to the discriminatedUnion is properly implemented.
56-63
: Appropriate transformation logic for coinpaprika configurationThe transformation logic for coinpaprika follows the same pattern as other providers, correctly extracting the necessary configuration values.
packages/pricing/test/factory/pricingFactory.spec.ts (2)
7-11
: Clean import organizationThe imports have been properly restructured to include CoinPaprikaProvider alongside the other imports from internal.js.
41-53
: Well-implemented test for CoinPaprikaProvider creationThe test follows the same pattern as other provider tests, ensuring consistency in the test suite. It correctly verifies that the factory creates an instance of CoinPaprikaProvider with the provided configuration.
apps/processing/src/config/env.ts (4)
36-36
: Proper addition of "coinpaprika" to PRICING_SOURCE enumThe PRICING_SOURCE enum has been correctly extended to include the new "coinpaprika" option.
58-64
: Well-structured schema for coinpaprika pricing configurationThe coinpaprikaPricingSchema follows the same pattern as other pricing provider schemas. The API type enumeration is comprehensive and includes appropriate defaults.
76-80
: Correct update to discriminatedUnion for validation schemaThe discriminatedUnion has been properly updated to include the new coinpaprikaPricingSchema.
86-93
: Appropriate transformation logic for coinpaprika configurationThe transformation logic for coinpaprika follows the established pattern, correctly extracting the necessary configuration values.
packages/pricing/src/interfaces/pricingConfig.interface.ts (2)
14-18
: Add CoinPaprika pricing config: looks good!The new type definition for
CoinPaprikaPricingConfig
is consistent with the existing pattern for other providers. The properties appear to match CoinPaprika requirements accurately.
24-26
: Extended union type for 'coinpaprika'Including the new
"coinpaprika"
branch in thePricingConfig
type ensures the factory can handle the new provider seamlessly without breaking existing logic.packages/pricing/src/providers/coinpaprika.provider.ts (12)
1-25
: Imports and initial type definitionsThe Axios setup and shared imports look good. No immediate syntax or logical issues are apparent here.
27-31
: CoinPaprikaOptions typeDefining
CoinPaprikaOptions
withapiKey
and theapiType
union covers all tiers mentioned in the.env.example
. Looks well aligned with usage in the rest of the code.
32-61
: Token mapping considerationProviding default mappings is helpful. For any undefined token (e.g.,
eBTC
,WLYX
), ensure the error-handling logic alerts users that these tokens are unsupported. This is already covered in exceptions, so it's consistent.
62-101
: Time constants and tier limitsUsing
Infinity
for some tiers is a straightforward way to represent no limit. Confirm that any code handling these tiers gracefully interpretsInfinity
. Otherwise, the approach is sound.
102-129
: Class definition and constructorThe constructor correctly sets up the Axios instance depending on whether an API key is required. Also, the acceptance of a logger is consistent with other providers. Ensure you avoid logging the API key or other sensitive data beyond this scope.
131-141
: getBaseUrl methodUsing a switch to differentiate free vs. paid endpoints is clear. If additional API tiers emerge, remember to update the switch accordingly.
143-175
: determineInterval methodThe interval logic is sensible given tier limits. Verify that falling back to
"1d"
for older data is acceptable for your use case, as it may return less granular data.
195-246
: getTokenPrice methodBy returning only the first price (
prices.at(0)
), ensure this aligns with your business requirement (whether you prefer earliest, latest, or average within the specified range). Everything else seems consistent.
248-302
: getTokenPrices methodSorting timestamps and filtering based on tier limits is a straightforward approach. The method then delegates the retrieval to
getTokenPricesWithBatching
. Overall, this is neatly structured.
304-342
: getTokenPricesWithBatching methodImplementing a 90-day batch size to prevent overly large queries is sensible. Check whether adding 1ms (line 327) between batches could leave a trivial gap. Often it's useful to allow a small overlap if you'd prefer continuous coverage.
344-389
: getHistoricalPrices methodThe API call logic is robust. It gracefully returns an empty array when no data is found. The mapping of response data to internal
TokenPrice
objects is straightforward and correct.
391-449
: handleAxiosError methodThe error-handling strategy comprehensively covers rate limits, auth issues, and server errors. Great job logging context and throwing meaningful exceptions.
COINPAPRIKA_API_KEY={{YOUR_KEY}} | ||
COINPAPRIKA_API_TYPE=pro # 'free' or 'starter' or 'pro' or 'business' or 'enterprise' |
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.
💡 Verification agent
🧩 Analysis chain
Well-documented configuration for new provider
The new environment variables are clearly defined with helpful comments about possible values for the API type.
🏁 Script executed:
#!/bin/bash
# Verify that similar environment variables are added to all relevant .env example files
# to maintain consistency across the project
echo "Checking for consistent environment variable definitions across .env files..."
fd -t f ".env.example" --exec grep -l "PRICING_SOURCE" {} \; | xargs grep -l "coingecko" | xargs grep "coinpaprika" || echo "Not all .env.example files include coinpaprika as a PRICING_SOURCE option"
fd -t f ".env.example" --exec grep -l "COINPAPRIKA_API_KEY" {} \; | wc -l
Length of output: 511
Action Required: Ensure Consistent Coinpaprika Configuration Across All .env.example Files
The documentation in scripts/bootstrap/.env.example
is clear and helpful. However, our consistency checks indicate that the new Coinpaprika variables aren’t present in all .env.example
files:
- The verification script found that none of the
.env.example
files include theCOINPAPRIKA_API_KEY
definition. - Additionally, the expected Coinpaprika option under the
PRICING_SOURCE
setting is missing in some files (the script report “Not all .env.example files include coinpaprika as a PRICING_SOURCE option”).
Please update all relevant .env.example
files to include the Coinpaprika configuration settings (e.g., COINPAPRIKA_API_KEY={{YOUR_KEY}}
and COINPAPRIKA_API_TYPE=pro …
) and ensure the PRICING_SOURCE
options list includes Coinpaprika for consistency across the project.
Description
Add Coinpaprika pricing provider
Checklist before requesting a review
Summary by CodeRabbit