Unified sdk bullish nightingdale batching #288
Conversation
…isk, supporting use case for referencing original data sources in conversation stream
…nd added verified the integration with event service
|
|
||
| ### 1. Telegram Ecosystem Integration | ||
|
|
||
| The SDK provides specialized support for Telegram applications: |
There was a problem hiding this comment.
what specialised support? as far as i know there is nothing telegram specific in SDK
| const { cid } = await this.ddcClient.store(this.bucketId, content); | ||
|
|
||
| // Record content hash on blockchain | ||
| const tx = this.blockchain.ddcCustomers.setBucketParams(this.bucketId, { |
There was a problem hiding this comment.
export type BucketParams = {
isPublic: boolean;
};
there is no metadata with contentCid etc.
| 'https://storage-3.mainnet.cere.network', | ||
| ], | ||
| }, | ||
| activity: { |
There was a problem hiding this comment.
do we really have anything deployed on https://api.stats.cere.network and https://search.cere.network?
Might be it's better to move code pieces into examples directory (that already exists) and make sure they are 100% working?
e.g. there is guide how to execute examples: https://github.com/Cerebellum-Network/cere-ddc-sdk-js/blob/9f634e6d736ca601fa1737bb0a373c53eeead6b8/examples/node/README.md
| end | ||
|
|
||
| subgraph "Data Layer" | ||
| IPFS[IPFS Compatible] |
There was a problem hiding this comment.
what is IPFs compatible?
as far as you now have much more context on system components, could you please go through this doc and make sure there is no trash that is actually not truth and could confuse and people who would learn this docs in details?
|
|
||
| ## Common Use Cases | ||
|
|
||
| ### Use Case 1: Telegram Bot with Quest System |
There was a problem hiding this comment.
is really Telegram Bot with Quest System a 'common use case' ?)
| } | ||
| ``` | ||
|
|
||
| ## Deployment Considerations |
There was a problem hiding this comment.
deployment of what? SDK? or some service that developers integrate SDK to?
i think should be removed
| end | ||
|
|
||
| subgraph "External Services" | ||
| ActivitySDK[Activity SDK] |
There was a problem hiding this comment.
is Activity SDK - external service?
Also this diagram very similar to diagram in cere-ddc-sdk-js-ecosystem-integration.md
I think there can be one 'overview' document (or even moved to main Readme.md) + usage/integration doc but need somehow to be aligned with existing 'examples' directory. Either follow the existing approach or align those example to new.
| bucketId: BigInt(12345), | ||
| network: 'testnet', | ||
| }, | ||
| activityConfig: { |
There was a problem hiding this comment.
i would suggest to rename ddcConfig and activityConfig just to ddc and activity
or if you vote for 'config' postfix, then there should be 'processingConfig' and 'loggingConfig'
main concern here - consistent naming
| timestamp: new Date(), | ||
| }; | ||
|
|
||
| const result = await sdk.writeTelegramEvent(questEvent, { |
There was a problem hiding this comment.
what's different between Telegram, and regular event?
Is separate method needed?
| timestamp: new Date(), | ||
| }; | ||
|
|
||
| const result = await sdk.writeTelegramMessage(message, { |
There was a problem hiding this comment.
looks like those methods are not aligned with the idea of 'unified' SDK
| interface UnifiedResponse { | ||
| transactionId: string; | ||
| status: 'success' | 'partial' | 'failed'; | ||
| dataCloudHash?: string; |
There was a problem hiding this comment.
what is dataCloudHash?
Is it CID?
| }, | ||
|
|
||
| // Nightingale-specific optimizations | ||
| nightingaleConfig: { |
There was a problem hiding this comment.
nightingale is one of the usecases. it shouldn't be in SDK config
| }, | ||
|
|
||
| // Performance tuning | ||
| performance: { |
There was a problem hiding this comment.
timeouts are not really about performance.
e.g. when we configure HTTP or GRPC server, it's not 'performance' section.
| processing: { | ||
| dataCloudWriteMode: 'direct', | ||
| indexWriteMode: 'realtime', | ||
| priority: 'high', |
There was a problem hiding this comment.
how priority works? didn't found explanation for it
| - [UnifiedSDKConfig](#unifiedsdkconfig) | ||
| - [UnifiedResponse](#unifiedresponse) | ||
| - [ProcessingMetadata](#processingmetadata) | ||
| - [TelegramEventData](#telegrameventdata) |
There was a problem hiding this comment.
I think Telegram, Bullish and Nightingale are special cases but our SDK is general purpose that can be used by other devs and actually the repo is open source.
From my point of view, any mentioning of Telegram, Bullish and Nightingale (keywords) should be removed from both code and docs
| }, | ||
| activityConfig: { | ||
| endpoint: 'https://api.stats.cere.network', | ||
| keyringUri: 'your mnemonic phrase here', // UriSigner compatible |
There was a problem hiding this comment.
UriSigner is one of the options.
E.g. in BullishApp we use CereWalletSigner.
so it should be possible to pass a Signer interface implementation
|
|
||
| // ✨ Nightingale Video Stream - Auto-detected | ||
| const result3 = await sdk.writeData({ | ||
| droneId: 'drone_001', |
There was a problem hiding this comment.
shouldn't we follow Event structure? also we could have a strong typing so that writeData accepts e.g. either file or event
| }); | ||
| ``` | ||
|
|
||
| ## Migration Guide |
There was a problem hiding this comment.
there was never writeTelegramEvent or writeBullishCampaign so not clear what this Migration Guide is about
| payload: this.transformPayloadForDDC(payload), | ||
| options: { | ||
| batchOptions: rules.additionalParams.batchOptions, | ||
| encryption: rules.additionalParams.encryption, |
There was a problem hiding this comment.
do we have encryption implemented? how to pass any encrpyption params like encryption key or anything else?
| encryption: rules.additionalParams.encryption, | ||
| ttl: rules.additionalParams.ttl, | ||
| ...(this.isBullishCampaign(payload) && { | ||
| campaignTracking: true, |
There was a problem hiding this comment.
what is campaignTracking?
| */ | ||
| private transformPayloadForDDC(payload: any): any { | ||
| // For Telegram events, create a DagNode structure | ||
| if (this.isTelegramEvent(payload)) { |
There was a problem hiding this comment.
no telegram specific logic
| } | ||
|
|
||
| // For Bullish campaigns, create a DagNode structure with campaign metadata | ||
| if (this.isBullishCampaign(payload)) { |
There was a problem hiding this comment.
no bullish specific logic
| metadata: { | ||
| type: 'bullish_campaign', | ||
| campaignId: payload.campaignId, | ||
| eventType: payload.eventType, |
There was a problem hiding this comment.
why do we need this fields in metadata?
| } | ||
|
|
||
| // For Nightingale video streams, create multipart structure | ||
| if (this.isNightingaleVideoStream(payload)) { |
There was a problem hiding this comment.
no Nightingale specific logic
|
|
||
| try { | ||
| // Initialize DDC Client with network preset | ||
| const networkConfig = |
There was a problem hiding this comment.
is mainnet supported somehow?
| response = await this.executeActivityAction(action); | ||
| break; | ||
|
|
||
| case 'http-api': |
There was a problem hiding this comment.
what action can we do using http-api?
| if (!this.activityClient) { | ||
| this.logger('warn', 'Activity SDK not available - skipping activity action'); | ||
| // Return a mock response to maintain workflow continuity | ||
| return { |
There was a problem hiding this comment.
shouldn't there be an error?
| priority: action.priority, | ||
| }; | ||
|
|
||
| return await this.executeActivityAction(batchAction); |
There was a problem hiding this comment.
inside executeDDCACtion we call executeActivityAction. smth is wrong
| /** | ||
| * Execute HTTP API action | ||
| */ | ||
| private async executeHTTPAction(action: Action): Promise<any> { |
| * Check if action is campaign-related | ||
| */ | ||
| private isCampaignAction(action: Action): boolean { | ||
| return action.options?.campaignTracking || action.options?.questTracking || action.options?.campaignId; |
There was a problem hiding this comment.
questTracking options should be removed
| /** | ||
| * Process campaign-specific logic after action execution | ||
| */ | ||
| private async processCampaignSpecificLogic(action: Action, response: any): Promise<void> { |
There was a problem hiding this comment.
not sure there should be any campaign specific logic
| /** | ||
| * Process quest update logic | ||
| */ | ||
| private async processQuestUpdate(payload: any, response: any): Promise<void> { |
| /** | ||
| * Update campaign metrics | ||
| */ | ||
| private async updateCampaignMetrics(payload: any, response: any): Promise<void> { |
| private mapDataCloudWriteMode(mode: string): ProcessingRules['dataCloudAction'] { | ||
| switch (mode) { | ||
| case 'direct': | ||
| return 'write_direct'; |
There was a problem hiding this comment.
instead of write_direct would be better to support 2 parameters.
e.g.
op: 'write'
mode: 'direct'
instead of merging this 2 parameters into one
| @@ -0,0 +1,224 @@ | |||
| # UnifiedSDK Test Suite | |||
There was a problem hiding this comment.
previosuly in SDK we used to write e2e tests that run against real infra running in docker compose.
The tests are here: https://github.com/Cerebellum-Network/cere-ddc-sdk-js/tree/main/tests/specs
at least UnifuedSDK spec should be added there, and might be there is no need to have so much unit tests and mocks but rather cover all main scenarious/use cases in that e2e spec.
It would be much easier to support.
Ofc if any unit test make sense to you, you can keep it. But i would suggest to follow existing naming convention. .spec.ts instead of .test.ts
- might be better to keep spec.ts next to file that we test instead of separate folder. would be easier to navigate from component to it's testing class. I don't insist on this too much, just shared thought
| jest.mock('../../Dispatcher'); | ||
| jest.mock('../../Orchestrator'); | ||
|
|
||
| describe('UnifiedSDK - Performance & Integration', () => { |
There was a problem hiding this comment.
performance test that uses mocks inside. Does it really make sense?
Not clear what exactly this performance test should verify, and taking into consideration the fact that there are mocks for all components under the hood...
2 participants
No description provided.