hw-health-charm

Merge ~xavpaice/charm-hw-health:lp1833897 into ~nagios-charmers/charm-hw-health:master

  1. Git
  2. lp:~xavpaice/charm-hw-health
  3. lp1833897
  4. Merge into master
Proposed by Xav Paice
Status: Merged
Approved by: Zachary Zehring
Approved revision: 76116e2700441159c9b7da7435753162c31f9731
Merged at revision: 8f438b16ee8d545890ed114c3fae1f9333a60c70
Proposed branch: ~xavpaice/charm-hw-health:lp1833897
Merge into: ~nagios-charmers/charm-hw-health:master
Diff against target: 14 lines (+1/-1)
1 file modified
src/files/ipmi/cron_ipmi_sensors.py (+1/-1)
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Needs Information
Canonical IS Reviewers Pending
Review via email: mp+369219@code.staging.launchpad.net

Commit message

Fix for lp1833897 handle separate filesystem for /var

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Stuart Bishop (stub) wrote :

I think this should be using shutil.move instead of shutil.copyfile

review: Needs Information
Revision history for this message
Stuart Bishop (stub) wrote :

Discussed this and I don't think we want to use either shutil.move or shutil.copyfile, as they don't provide atomicity guarantees and nrpe can trip over an incomplete file.

Instead, we should ensure the temporary file is on the same filesystem as the destination and stick with os.rename. The easiest way is to set TMP_OUTPUT_FILE to OUTPUT_FILE + ".tmp" instead of using the module global. Or use tempfile.TemporaryNamedFile(delete=False,dir=os.path.dirname(OUTPUT_FILE))

Revision history for this message
Xav Paice (xavpaice) wrote :

Many thanks for that, I've updated the change accordingly. This does seem like a better approach.

Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 8f438b16ee8d545890ed114c3fae1f9333a60c70

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches