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

Make sure the callback function is called only once #632

Closed
wants to merge 1 commit into from

Conversation

@ab9-er
Copy link
Contributor

@ab9-er ab9-er commented Dec 3, 2015

Since the callback is called before checking the size of data chunk, the callback function is called twice.

@ab9-er ab9-er mentioned this pull request Dec 3, 2015
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Dec 6, 2015

Can you elaborate on why this is problematic in your case? I'm assuming your callback is doing something other than the typical "progress bar/percentage/etc" type output, but offhand I'm not sure why having the callback called 2x in a row with the same size value would actually break things.

Seems like an innocent enough change but it also feels like one of those spots where somebody else may be relying on this only-sort-of-buggy behavior, somehow.

@ab9-er
Copy link
Contributor Author

@ab9-er ab9-er commented Dec 7, 2015

Consider this log,

[INFO] - [03/12/2015 06:37:18 PM] : 43% transfer completed
[INFO] - [03/12/2015 06:37:18 PM] : 87% transfer completed
[INFO] - [03/12/2015 06:37:18 PM] : 100% transfer completed
[INFO] - [03/12/2015 06:37:18 PM] : File transfer from SFTP to local TEMP complete
[INFO] - [03/12/2015 06:37:18 PM] : 100% transfer completed
[INFO] - [03/12/2015 06:37:18 PM] : File transfer from SFTP to local TEMP complete

In my trivial case, the logger is called twice. In case, if some other function is called it could result in errorneous behavior.

@bitprophet bitprophet added this to the 1.16.1 milestone Dec 7, 2015
bitprophet added a commit that referenced this issue Apr 24, 2016
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Apr 24, 2016

Backported to 1.15.x, added test, refactored (I think you probably meant to update putfo too, but it got missed :D now that won't be a problem anymore...), added changelog, should be all set now. Thanks!

@ab9-er
Copy link
Contributor Author

@ab9-er ab9-er commented May 12, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants