-
Notifications
You must be signed in to change notification settings - Fork 239
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
[Main][Task]26451789: Add Offline Support #2241
Conversation
_maxBatchInterval = storageConfig.maxSentBatchInterval; | ||
_maxBatchSize = storageConfig.maxBatchsize; | ||
} | ||
_urlCfg = urlConfig; |
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.
Add todo: this will overwrite any "previous" if the endpoint is changed as you construct a "new" one. Which means all of the previous stuff will either get lost or left behind in memory -- because it will never be cleaned up on teardown.
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.
created task 26681188 for this
|
||
public _appId: string; //TODO: set id | ||
|
||
constructor(endpointUrl?: string) { |
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.
Add a TODO: to remove the passing (and internal usages) of the endpointURL, as your already always creating without an endpoint anyway.
If there is no other endpoint (the URL) lets just not send anything
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.
Actually, lets add a TODO to remove this class completely so we can delegate the "sending" to the real channels via a new "sendbatch" (or something like that) on IOfflineSupport, that way we don't need to worry about all of the transports etc)
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.
Endpoint is removed, and if no url is provided, doOncomplete
will be called with status code 400 and will throw internal as well.
created Task 26681160 for this to add more tests
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.
Lets push this change in and we can address at least the timer stuff in a follow up PR (before release)
TODO: add security doc to let users know that data would be stored