-
Notifications
You must be signed in to change notification settings - Fork 1
Implement translations from E2DR to DR2E #60
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: main
Are you sure you want to change the base?
Conversation
7777ab8 to
bd84e89
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.
@GasperSenk we need to wait for changes in the snap-in manager before we can promote this right?
7d0c9ad to
b02e9db
Compare
b02e9db to
6fc6691
Compare
|
There's some backward-incompatible changes due to changes inside of the enum values. |
…d names to the new ones.
…mount of changes needed on snap-in side.
9ec0f35 to
1c8b9e6
Compare
|
These changes have been tested against a snap-in and show no regressions. |
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.
Pull Request Overview
This PR migrates event type naming from the old E2DR (Extraction-to-DevRev) convention to the new DR2E (DevRev-to-Extraction) convention. The changes introduce a translation function to handle backward compatibility with legacy event type names while updating all enum values to the new naming scheme.
Key Changes:
- Added
getEventType()translation function with mapping table for old-to-new event type names - Updated all
EventTypeandExtractorEventTypeenum values to new DR2E naming convention - Applied translation at spawn entry point to convert incoming events to new format
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/common/helpers.ts | Implements translation function and mapping table for event type migration |
| src/types/extraction.ts | Updates enum values from E2DR to DR2E naming convention, adds UnknownEventType |
| src/workers/spawn.ts | Applies event type translation at spawn entry point |
| src/tests/timeout-handling/*.test.ts | Updates test expectations to use enum constants instead of string literals |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Lets write down from which version of the SDK, the SDK will accept new event types. It can start returning new event types now as the gateway is already translating them. |
| initialDomainMapping, | ||
| options, | ||
| }: SpawnFactoryInterface<ConnectorState>): Promise<void> { | ||
| event.payload.event_type = getEventType(event.payload.event_type); |
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.
Did you check if this is the only place where we need to do this translation?
Also, do you think it is worth creating some transform/translateEvent function which transforms the whole event in new event with some fields edited or structure changed a bit (in case we need it in future)?
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.
Let's also mark with comment (or add TODO) that this is temporary change and will be removed once SIM starts sending and receiving only new event types.
| * 2) Valid DR2E names are returned as-is | ||
| * 3) Unknown values return `UnknownEventType` | ||
| */ | ||
| export function getEventType(eventType: string): EventType { |
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.
I would rename this to sth like transform/translateEventType, since we already have event type but need to translate it to the new one.
| ExtractionExternalSyncUnitsStart = 'START_EXTRACTING_EXTERNAL_SYNC_UNITS', | ||
| ExtractionMetadataStart = 'START_EXTRACTING_METADATA', | ||
| ExtractionDataStart = 'START_EXTRACTING_DATA', | ||
| ExtractionDataContinue = 'CONTINUE_EXTRACTING_DATA', | ||
| ExtractionDataDelete = 'START_DELETING_EXTRACTOR_STATE', | ||
| ExtractionAttachmentsStart = 'START_EXTRACTING_ATTACHMENTS', | ||
| ExtractionAttachmentsContinue = 'CONTINUE_EXTRACTING_ATTACHMENTS', | ||
| ExtractionAttachmentsDelete = 'START_DELETING_EXTRACTOR_ATTACHMENTS_STATE', | ||
|
|
||
| // Loading | ||
| StartLoadingData = 'START_LOADING_DATA', | ||
| ContinueLoadingData = 'CONTINUE_LOADING_DATA', | ||
| StartLoadingAttachments = 'START_LOADING_ATTACHMENTS', | ||
| ContinueLoadingAttachments = 'CONTINUE_LOADING_ATTACHMENTS', | ||
| StartDeletingLoaderState = 'START_DELETING_LOADER_STATE', | ||
| StartDeletingLoaderAttachmentState = 'START_DELETING_LOADER_ATTACHMENT_STATE', | ||
|
|
||
| // Unknown | ||
| UnknownEventType = 'UNKNOWN_EVENT_TYPE', | ||
| } | ||
|
|
||
| /** | ||
| * ExtractorEventType is an enum that defines the different types of events that can be sent from the external extractor to ADaaS. | ||
| * The external extractor can use these events to inform ADaaS about the progress of the extraction process. | ||
| */ | ||
| export enum ExtractorEventType { | ||
| // Extraction | ||
| ExtractionExternalSyncUnitsDone = 'EXTRACTION_EXTERNAL_SYNC_UNITS_DONE', | ||
| ExtractionExternalSyncUnitsError = 'EXTRACTION_EXTERNAL_SYNC_UNITS_ERROR', | ||
| ExtractionMetadataDone = 'EXTRACTION_METADATA_DONE', | ||
| ExtractionMetadataError = 'EXTRACTION_METADATA_ERROR', | ||
| ExtractionDataProgress = 'EXTRACTION_DATA_PROGRESS', | ||
| ExtractionDataDelay = 'EXTRACTION_DATA_DELAY', | ||
| ExtractionDataDone = 'EXTRACTION_DATA_DONE', | ||
| ExtractionDataError = 'EXTRACTION_DATA_ERROR', | ||
| ExtractionDataDeleteDone = 'EXTRACTION_DATA_DELETE_DONE', | ||
| ExtractionDataDeleteError = 'EXTRACTION_DATA_DELETE_ERROR', | ||
| ExtractionAttachmentsProgress = 'EXTRACTION_ATTACHMENTS_PROGRESS', | ||
| ExtractionAttachmentsDelay = 'EXTRACTION_ATTACHMENTS_DELAY', | ||
| ExtractionAttachmentsDone = 'EXTRACTION_ATTACHMENTS_DONE', | ||
| ExtractionAttachmentsError = 'EXTRACTION_ATTACHMENTS_ERROR', | ||
| ExtractionAttachmentsDeleteDone = 'EXTRACTION_ATTACHMENTS_DELETE_DONE', | ||
| ExtractionAttachmentsDeleteError = 'EXTRACTION_ATTACHMENTS_DELETE_ERROR', | ||
| ExtractionExternalSyncUnitsDone = 'EXTERNAL_SYNC_UNIT_EXTRACTION_DONE', | ||
| ExtractionExternalSyncUnitsError = 'EXTERNAL_SYNC_UNIT_EXTRACTION_ERROR', | ||
| ExtractionMetadataDone = 'METADATA_EXTRACTION_DONE', | ||
| ExtractionMetadataError = 'METADATA_EXTRACTION_ERROR', | ||
| ExtractionDataProgress = 'DATA_EXTRACTION_PROGRESS', | ||
| ExtractionDataDelay = 'DATA_EXTRACTION_DELAYED', | ||
| ExtractionDataDone = 'DATA_EXTRACTION_DONE', | ||
| ExtractionDataError = 'DATA_EXTRACTION_ERROR', | ||
| ExtractionDataDeleteDone = 'EXTRACTOR_STATE_DELETION_DONE', | ||
| ExtractionDataDeleteError = 'EXTRACTOR_STATE_DELETION_ERROR', | ||
| ExtractionAttachmentsProgress = 'ATTACHMENT_EXTRACTION_PROGRESS', | ||
| ExtractionAttachmentsDelay = 'ATTACHMENT_EXTRACTION_DELAYED', | ||
| ExtractionAttachmentsDone = 'ATTACHMENT_EXTRACTION_DONE', | ||
| ExtractionAttachmentsError = 'ATTACHMENT_EXTRACTION_ERROR', | ||
| ExtractionAttachmentsDeleteDone = 'EXTRACTOR_ATTACHMENTS_STATE_DELETION_DONE', | ||
| ExtractionAttachmentsDeleteError = 'EXTRACTOR_ATTACHMENTS_STATE_DELETION_ERROR', |
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.
Did you carefully (really carefully!!) verified all these names with ones in airdrop-snapin-manager?
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.
Also, is this renaming a breaking change? I know interface stayed the same but we are exporting this stuff, so what if someone relied on actual string values? I am thinking if we should rather create ExtractorEventTypeV2 and EventTypeV2, work with them and mark these as deprecated?
Summary
Migrated event type names from old E2DR to DR2E naming convention.
Added a simple function for easier translation and migration.
Connected Issues
Craftsmanship, Integrity, and Devil’s Advocacy
Story of the craft