Skip to content

Avoid unreference variable 'st' error if file was removed. Raise exception if unrecognized environment error from os.stat#413

Merged
josegonzalez merged 1 commit into
python-beaver:masterfrom
gregsterin:fix-unsassigned-variable-exception-on-removed-file
Sep 20, 2016
Merged

Avoid unreference variable 'st' error if file was removed. Raise exception if unrecognized environment error from os.stat#413
josegonzalez merged 1 commit into
python-beaver:masterfrom
gregsterin:fix-unsassigned-variable-exception-on-removed-file

Conversation

@gregsterin
Copy link
Copy Markdown
Contributor

Unit tests passing.
Did a manual test, and no crash, tailed file gets removed, and is added back if it appears again now.

Comment thread beaver/worker/tail.py Outdated
self._log_info('file removed')
self.close()
return
else:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for an else since you return already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@josegonzalez trying to return only if errorcode is for file not found, else raise the exception since I'd imagine other errors stat-ing the file would be unexpected.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, what I'm saying is remove the else and de-dent the raise.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok makes sense. Done.

…ption if unrecognized environment error from os.stat
@gregsterin gregsterin force-pushed the fix-unsassigned-variable-exception-on-removed-file branch from e3cd738 to 4f40120 Compare September 19, 2016 23:49
@gregsterin
Copy link
Copy Markdown
Contributor Author

@josegonzalez lmk if I need to add anything else. Would we be able to get this into an upcoming release?

@josegonzalez josegonzalez merged commit 31cabc6 into python-beaver:master Sep 20, 2016
@gregsterin
Copy link
Copy Markdown
Contributor Author

@josegonzalez just wondering, will there be a release of this soon? I need this change as currently during logrotate, sometimes this bug causes beaver to crash (old file rotated, new file not yet created hits this case). If you'll be waiting a while to do a release, I'll just have to pip install from github to get the change., which is fine. However, would be nice to have a new version :-).

@josegonzalez
Copy link
Copy Markdown
Member

I forgot to make a release because I was playing video games. Let me see what I can do.

@josegonzalez
Copy link
Copy Markdown
Member

josegonzalez commented Sep 20, 2016

Done, 36.2.1 was released.

@gregsterin
Copy link
Copy Markdown
Contributor Author

Thanks!!

@JamieCressey
Copy link
Copy Markdown
Member

#390

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.

3 participants