Skip to content

Migrate External Sync Units to file upload and add confirmation skip#156

Merged
gasperzgonec merged 11 commits intomainfrom
gasperz/ISS-265202
Mar 11, 2026
Merged

Migrate External Sync Units to file upload and add confirmation skip#156
gasperzgonec merged 11 commits intomainfrom
gasperz/ISS-265202

Conversation

@gasperzgonec
Copy link
Copy Markdown
Contributor

Description

These changes change the behavior of external sync unit upload.
This, instead of sending data inside of the event, stores the external sync units inside of an artifact that's uploaded and we only send the artifact list.

This change is done entirely behind the scenes, so no end-user changes are required.

Current batch size is set to 25000.

Connected Issues

Checklist

  • Tests added/updated and ran with npm run test OR no tests needed.
  • Ran backwards compatibility tests with npm run test:backwards-compatibility.
  • Code formatted and checked with npm run lint.
  • Tested airdrop-template linked to this PR.
  • Documentation updated and provided a link to PR / new docs OR no docs needed.

@gasperzgonec gasperzgonec requested review from a team and radovanjorgic as code owners March 9, 2026 14:19
Aakkash-Suresh
Aakkash-Suresh previously approved these changes Mar 9, 2026
Copy link
Copy Markdown

@Aakkash-Suresh Aakkash-Suresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean, all external sync units discovery are now propagated through Artifacts?

@gasperzgonec
Copy link
Copy Markdown
Contributor Author

That is correct, yes.

Copy link
Copy Markdown
Collaborator

@radovanjorgic radovanjorgic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added few comments. I am curious if the way how external sync units worker is written will change? Are we still going to support only passing external sync units array in event data and upload it as an artifact in SDK, or we plan to tell developers to start using repos for this? Also, we are ignoring event data length here compeltely (the logic that you have for tracking event data length and emitting if it is over 80%), can we do anything about it?

Comment thread src/multithreading/worker-adapter/worker-adapter.ts
Comment thread src/uploader/uploader.ts
Comment thread src/multithreading/worker-adapter/worker-adapter.ts Outdated
Comment thread src/multithreading/worker-adapter/worker-adapter.ts Outdated
@gasperzgonec
Copy link
Copy Markdown
Contributor Author

Added few comments. I am curious if the way how external sync units worker is written will change? Are we still going to support only passing external sync units array in event data and upload it as an artifact in SDK, or we plan to tell developers to start using repos for this? Also, we are ignoring event data length here compeltely (the logic that you have for tracking event data length and emitting if it is over 80%), can we do anything about it?

We aren't ignoring the event data length here. It's just different. Currently, our issue with event data length was due to the content of the message being sent to SQS being too large. Now we only send the list of files, which is this case will always be maximum of 5 items (the max limit is 125k and we have max 5 batches of 25k).

I would deprecate the current way, and migrate users to the new way of pushing to repo, but I wasn't sure on how to deprecate this.

radovanjorgic
radovanjorgic previously approved these changes Mar 11, 2026
Copy link
Copy Markdown
Collaborator

@radovanjorgic radovanjorgic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, maybe you could use that AirSyncDefaultItemTypes enum instead of that constant wherever needed in project.

…new const.

This exports item types, but keeps the internal type local to the code.
@gasperzgonec gasperzgonec merged commit c88d370 into main Mar 11, 2026
8 of 9 checks passed
@gasperzgonec gasperzgonec deleted the gasperz/ISS-265202 branch March 11, 2026 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants