kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

With 2.7 dropped, use shutil disk_usage function universally.

Open rtibbles opened this issue 10 months ago • 9 comments

Summary

Removes platform specific behaviour. Uses shutil disk_usage which has support on Unix systems and Windows. This may well break Android, as it removes the Android specific code, but this will be rectified in the Android installer with this issue: https://github.com/learningequality/kolibri-installer-android/issues/211

References

Fixes the Kolibri side of https://github.com/learningequality/kolibri-installer-android/issues/211

Reviewer guidance

Windows and Unix systems should show free space appropriately for content imports.

Opening as a draft to first confirm the impact on the Android installer.


Testing checklist

  • [ ] Contributor has fully tested the PR manually
  • [ ] If there are any front-end changes, before/after screenshots are included
  • [ ] Critical user journeys are covered by Gherkin stories
  • [ ] Critical and brittle code paths are covered by unit tests

PR process

  • [ ] PR has the correct target branch and milestone
  • [ ] PR has 'needs review' or 'work-in-progress' label
  • [ ] If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • [ ] If this is an important user-facing change, PR or related issue has a 'changelog' label
  • [ ] If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

rtibbles avatar Mar 30 '24 20:03 rtibbles

Hi @rtibbles - the app force closes immediately after launch on all of my devices. Here are the logs, db and logcat from Android studio:

logs-logcat.zip

pcenov avatar Apr 11 '24 13:04 pcenov

I guess that takes the uncertainty out of this!

This may well break Android

rtibbles avatar Apr 11 '24 14:04 rtibbles

Looking at the logs, this is the same error you saw on my PR: https://github.com/learningequality/kolibri/pull/11974 - and doesn't seem to be directly related to this PR at all.

rtibbles avatar Apr 11 '24 14:04 rtibbles

I will come back to this PR when the other one has been merged.

rtibbles avatar Apr 11 '24 14:04 rtibbles

This might need a rebase, since https://github.com/learningequality/kolibri/pull/11974 has been merged... 🤔 (tested the APK today, the initial app force close after launch is still extant)

radinamatic avatar May 24 '24 15:05 radinamatic

Have rebased on latest develop, so hopefully any errors should be purely its own!

rtibbles avatar May 24 '24 18:05 rtibbles

Comment Screenshhot
This latest APK installs and goes through the setup wizard without issues 🙂 Any specific reviewer guidance for this PR? 2024_05_24_23 08 39

radinamatic avatar May 24 '24 21:05 radinamatic

The only possible source of breakage would be the free space being misreported during resource import.

rtibbles avatar May 25 '24 05:05 rtibbles

Hi @rtibbles - no issues observed during resource import on the Android app and the 'Free disk space' value gets updated correctly.

pcenov avatar May 28 '24 14:05 pcenov