Skip to content

Conversation

@foxbunny
Copy link

On Windows, Python does not have a resource module, which causes django-stdimage to raise an ImportError (ModuleNotFoundError in Py3).

This patch is a workaround for the issue as discussed in #127. It wraps the import in try-except and disables memory usage stats in the progress bar when import fails.

Signed-off-by: Hajime Yamasaki Vukelic <hayavuk@gmail.com>
Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

Perfect, that was fast! I had one tiny comment 👍


class MemoryUsageWidget(progressbar.widgets.WidgetBase):
def __call__(self, progress, data):
if not resource:
Copy link
Owner

Choose a reason for hiding this comment

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

if resource is not None:

Copy link
Author

Choose a reason for hiding this comment

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

Ah, yes, that looks better. Will fix.

@codingjoe codingjoe self-assigned this Jan 26, 2017
First cover the more common path.

Signed-off-by: Hajime Yamasaki Vukelic <hayavuk@gmail.com>
@foxbunny
Copy link
Author

Branch updated. Hopefully this is along the lines of what you had in mind.

Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

Sorry, my bad, I wrote it wrong. Makes no sense to switch it around. It was just about testing explicitly that it is None.

return 'RAM: {0:10.1f} MB'.format(
resource.getrusage(resource.RUSAGE_SELF).ru_maxrss / 1024
)
if resource is not None:
Copy link
Owner

Choose a reason for hiding this comment

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

dang, I wrote it wrong.

if resource is None:
    return 'RAM: N/A'
return ...

Copy link
Author

Choose a reason for hiding this comment

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

Oh alright. It'll be some hours before I'll be able to switch those around (day job). Though, the your first suggestion is not necessarily incorrect since it's a more common branch, likely to be hit more often in real life. Let me know if you still want them flipped.

@codingjoe
Copy link
Owner

Don't worry I fixed it for you: f317a04

Released in 4.2.1

Thanks!!!

@codingjoe codingjoe closed this Jan 27, 2017
@foxbunny foxbunny deleted the fix/win-resources branch January 27, 2017 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants