Skip to content
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

ctlv3: close snapshot file before rename (Windows) #6454

Merged
merged 1 commit into from
Sep 16, 2016
Merged

ctlv3: close snapshot file before rename (Windows) #6454

merged 1 commit into from
Sep 16, 2016

Conversation

sinsharat
Copy link
Contributor

This issue was occurring since before renaming, the file was still open as defer was used which would have closed it at the end of the method. The same has been corrected now.
#6451

@gyuho
Copy link
Contributor

gyuho commented Sep 16, 2016

Does this fix Windows issue in your machine? And commit title? Thanks!

@sinsharat
Copy link
Contributor Author

sinsharat commented Sep 16, 2016

@gyuho yes, after testing only i have submitted the fix. Its a windows behaviour, if a file is open, then it cannot be renamed or moved or deleted.
Proof attached.
image

Thanks!

@sinsharat
Copy link
Contributor Author

@gyuho what do you want me to change the commit title to?

@gyuho
Copy link
Contributor

gyuho commented Sep 16, 2016

@sinsharat Yes please. Thanks!

@sinsharat
Copy link
Contributor Author

@gyuho actually i wanted your opinion on what to keep the title.

@gyuho
Copy link
Contributor

gyuho commented Sep 16, 2016

@sinsharat How about

ctlv3: close snapshot file before rename (Windows)

@sinsharat sinsharat changed the title etcdctl/ctlv3: windows save snapshot issue fixed ctlv3: close snapshot file before rename (Windows) Sep 16, 2016
@sinsharat
Copy link
Contributor Author

@gyuho changed.
Again the CI failed due to failure of loading packages :(. Any reason for this.
Thanks for suggesting an appropriate title.

@gyuho
Copy link
Contributor

gyuho commented Sep 16, 2016

@sinsharat Please also change the commit title in git.

Tests might fail for different reasons, but not from this patch.

Thanks!

@sinsharat
Copy link
Contributor Author

@gyuho changed the title in git.
Thanks!

@xiang90
Copy link
Contributor

xiang90 commented Sep 16, 2016

lgtm

@xiang90 xiang90 merged commit 86aeeca into etcd-io:master Sep 16, 2016
@sinsharat sinsharat deleted the windows_save_snapshot_fix branch September 17, 2016 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants