-
Couldn't load subscription status.
- Fork 9
Move tests to version 3.8 + fix tempdir handling in older python #271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ake it compatible with older pythons
|
We can discuss if run tests against 3.7 or 3.8 version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally it looks ok.
But I do not quite like that we do these changes to support that old version of the Python. Seems like we create technical debt on purpose
mergin/utils.py
Outdated
| tmp_dir.cleanup() | ||
| except PermissionError: | ||
| mp.log.warning(f"Permission error during tmp dir cleanup: {tmp_dir.name}") | ||
| pass # Ignore the error and continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the pass here and also below is not necessary, there is another statement in the except block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we support QGIS versions 3.22+ we can not use concepts from Python versions 3.9+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😶🌫️ true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are few more places where the explicit cleanup of temporary dir should be added:
- in download_project_cancel()
- in download_project_finalize() when raising exceptions
- in similar calls for pull and push
Try to look at this in commit @wonder-sk , thanks |
Uh oh!
There was an error while loading. Please reload this page.