-
Notifications
You must be signed in to change notification settings - Fork 369
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
fix!: remove configstore #1871
fix!: remove configstore #1871
Conversation
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.
Such a therapeutic PR
I think |
* fix: remove configstore * removed unused varaibles * remove xdg-basedir
Hi @rhodgkins you are correct, with the removal of config store as a requirement any environment variables previously used to set the path for config store to write to are no longer necessary. |
Thanks @ddelgrosso1, out of interest how do resumable uploads work now? Are they only resumable for the lifetime of the nodejs process? |
@rhodgkins apologies for the delay in responding. Yes that is correct they are only resumable during the lifetime of a process now. We realized during our investigation into whether or not configStore was needed that there existed the possibility of data corruption in the old methodology. ConfigStore was only being used to store the first 16 bytes of an uploaded file which is a poor way to determine if the files are the same. Coupled with the numerous issues with having to cache on disk we made the decision to remove configStore entirely. Now, during the lifetime of a process a resumable upload will resume from the spot of interruption. |
Fixes: googleapis/gcs-resumable-upload#347