Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Adds docs about using ProgressBar with threads #2871

Merged
merged 1 commit into from Dec 5, 2016

Conversation

dkliban
Copy link
Member

@dkliban dkliban commented Nov 30, 2016

No description provided.

@mention-bot
Copy link

@dkliban, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bmbouter, @goosemania and @seandst to be potential reviewers.

@dkliban
Copy link
Member Author

dkliban commented Nov 30, 2016

Add test.py[0] to your pulp/app/pulp/app directory and start the django shell. Then run

import test

[0] https://gist.githubusercontent.com/dkliban/bc40c6cb28e5ab3cbedeb56bff6d55b0/raw/28ea46d12c297410f74449bc96cacbb0f00e6279/test.py

@bmbouter bmbouter changed the title [WIP] Makes ProgressBar.increment() method thread safe Makes ProgressBar.increment() method thread safe Nov 30, 2016
@bmbouter
Copy link
Member

I removed [WIP] from the title since we us the Label now for that

@bmbouter
Copy link
Member

@dkliban I did use the reproducer and I did reproduce the issue. I experimented with several other options and read some documentation. I've come to the conclusion that what is really important is that all threads use the same instance of ProgressBar and then we don't have to worry about thread race conditions inside of the instance methods.

I can write more on this tomorrow and we can discuss it some too with @mhrivnak who originally raised the thread safety concern. We likely will add some doc lines to the docstring, but I don' think a code change is necessary here.

@dkliban dkliban force-pushed the thread-safe-progress branch 14 times, most recently from 428802d to 30ad9e2 Compare December 4, 2016 18:21
@dkliban dkliban changed the title Makes ProgressBar.increment() method thread safe Adds documentation about using ProgressBar with threads Dec 4, 2016
@dkliban dkliban changed the title Adds documentation about using ProgressBar with threads Adds docs about using ProgressBar with threads Dec 4, 2016
@bmbouter
Copy link
Member

bmbouter commented Dec 5, 2016

What about this?

When using threads to update a ProgressBar in parallel, it is recommended all threads share the same in-memory instance. Django does not synchronize in-memory model instances, so multiple instances of a specific ProgressBar will diverge as they are written to in separate threads.


When using threads to update a ProgressBar in parallel, it is recommended all threads share the
same in-memory instance. Django does not synchronize in-memory model instances, so multiple
instances of a specific ProgressBar will diverge as they are written to in separate threads.
Copy link
Member

Choose a reason for hiding this comment

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

s/in/from/

Copy link
Member

@bmbouter bmbouter left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks @dkliban !

@dkliban dkliban merged commit c724040 into pulp:3.0-dev Dec 5, 2016
@dkliban dkliban deleted the thread-safe-progress branch December 5, 2016 21:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants