-
Notifications
You must be signed in to change notification settings - Fork 143
Preliminary python 3.6 port #2564 #2565
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
Preliminary python 3.6 port #2564 #2565
Conversation
As part of our move to Python 3 we must first establish our new python 3 dependencies via Poetry. And resolve all issues re dependencies from there on.
|
From the following: we may just have an incomplete dev install for our new Python 3.6. |
and re-running the |
|
A subsequent build.sh attempt using the new Python 3.6 .venv indicates an issue with Django settings: Porting P2 to P3 reference: https://portingguide.readthedocs.io/en/latest/numbers.html#the-l-suffix-not-allowed-in-numeric-literals |
Python 3 no longer uses the "L" suffix:
|
subsequent |
|
I am currently chasing down all our old-style imports where we now need to include the current directory name. |
Python 3 has more stringent import path requirements.
|
Post the last commit re full paths in imports for Python 3 compatibility we have our initial build.sh execution. |
|
Embarking on initial string conversion now as Python 3 does things differently.
On formatting enhancements in this area we also now have f stings in 3.6 and onwards:
|
Python 3 has more stringent import path requirements.
md5.update(l) TypeError: Unicode-objects must be encoded before hashing
|
This is great progress @phillxnet! |
|
@Hooverdan96 Cheers, just wanted to get some leg-work done on this so we can start spinning off more focused issues. |
|
I am actually really surprised that you got away with not changing any of the other package versions (chardet, gevent, etc.) since some of these have not even had an official release in a couple of years (like django-braces, gevent-websocket or urlobject and our current version is still below the last "old" one). That seems encouraging, so you can break this into smaller changes at a time. |
|
@Hooverdan96 Re:
We are not actually there yet thought: We may yet have issues down the way, but fingers cross we may yet pull this off with our existing pinned versions. "gevent-websocket" yes that one is a worry. Currently I'm battling with a simple file open not working as expected. But given some more patience we may yet find out more about how we have to change our existing code. Lots of suttle changes. If I get really stuck I'll post and we can sort out what obvious thing I'm missing. But currently just familiarising myself with some of the changes we may need here code wise. Still quite a way from much working as-is. |
There was a subtle file pointer behaviour change between 2.7 and 3.6 which broke this mechanism. Address TODO re custom opener re Python 3.
|
OK, that set of bits (commit) took ages but I had to track down a 'strangeness' that was not there in 2.7. Ended up doing a tidy and prior TODO of mine to address and we now have a working initrock.py script (rockstor-pre.service): with our new-shiny .venv (for Leap 15.4): |
|
And we have a working rockstor.service as well apparently !! Not sure I believe that one just yet actually. |
|
rockstor-bootstrap.service !!! We must have a myriad of string handling issues still but that is quite encouraging at least. But I've had to take out our prior env hack on run_comand() so will have to circle back around on that one. Reboot gives same result on services: I better sit down I think. |
|
@Hooverdan96 Re:
A little more on this, I just trying to minimise the scope of change - at least initially. I.e. changing our underlying language version seemed plenty large enough initially: mixing that with every individual dependency changing seemed like asking for trouble. We already here have a change in the included email module from the language change so lets see where we go initially with just the language change. Almost all of our dependencies are already Python 3.6 compatible (if Poetry is doing it job) so let see. I'm still getting to grips with the new string handling which will likely keep cropping up. This PR is intended as a proof of concept for just jumping the language version up. If it pans out I'll write it up and pop it in as say "Initial Python 3.6 migration" pull request and we can then, as you say, spin off the various problems found there-after. |
|
System - Services (TypeError):
The previously build-in cmp() has been dropped by Python 3: And we have a compound issue here with our use of old style sorted():
|
"cmp" sorted parameter and buildin function dropped in Python 3.
|
System - Users (Type Error)
|
"cmp" sorted parameter and builtin function dropped in Python 3. Partial fix as only Admin users showing - suspect unrelated issue.
|
System - Group ( Expected object of type bytes or bytearray, got: <class 'str'> )
https://pypi.org/project/chardet/ https://github.com/chardet/chardet/releases/tag/5.0.0 Updated to 5.0.0 while we are here, just in case:
|
|
Touching on our own chardet dependency here, with the last comments observed caveat (where upgrading for us may break for PySocks) we could now (post Python 3.6) side-step this depending instead and move to: charset-normalizer - which was brought in indirectly by Poetry to serve currently certifi [package.dependencies] https://pypi.org/project/charset-normalizer/ Python >=2.7,<3.5: Unsupported |
|
Turns out we are the only remaining user of chardet so dropping in favour of charset-normalizer which is already used by our requests dependency. Not sure of prior mention of chardet on PySocks. |
Replace hard chardet dependency with soft charset-normalizer also used by requests. Removes redundant encoding on groupname already encoded.
|
@Hooverdan96, I believe the removal has already begun. See b56967c, where it has now been removed from the list of dependencies and the code started to be adjusted accordingly. |
|
Ah, ok. I thought it would be replaced with charset-normalizer instead, which would then perform the same function. I guess, my "real" question was whether the recognition of a given encoding is really relevant for Rockstor in general ... |
You are correct; although it doesn't seem to be a drop-in replacement either. I'm not sure about that exactly, so @phillxnet will confirm/correct.
This is where I'll show even more my limits. To the best of my knowledge, it is important for those users with different locales and characters that would otherwise not be understood/read properly; that would of course be a big deal for user names, for instance. |
|
ah, yes, I think you're right. Obviously, the encoding matter for anything that is entered manually like share names, user/login, etc. Got it. |
…_runtime Move UDEVADM and SHUTDOWN definitions to runtime constants rockstor#2564
…poetry.lock_for_python_3.6 Move passwd to str and use crypt.mksalt() to generate salt rockstor#2564
|
Linking too two now merged (into this working branch) pull requests from @FroggyFlox |
|
Having now intergrated @FroggyFlox changes I'm now following up on more @FroggyFlox exploration into our not haveing any non admin users: |
|
Side channel chat suggests we move to subprocess.run() in the system.users.get_users() due to changes in open and our new python 3 base. |
Thanks to @FroggyFlox for investigations here.
Move to more modern - time limited subprocess.run().
|
Non admin users are now found as expected. Remaining Dashboard issues:
|
|
Planned initial P3 fix is configuring Samba results in: |
I may have figured that one out: [@phillxnet EDIT] Proposed for dedicated issue post "Preliminary python 3.6 port". |
In Python 3 we use iter(d.items()) instead: https://peps.python.org/pep-0469/
|
Initial working samba config via:
|
|
scheduled snapshot configurable and ends up running but Web_UI has no knowledge of these runs: |
|
System - Logs manager - Logs ReaderNon functional: i.e. no tail type output. Logs Downloader :: compressed packet builderLooks to work as intended with working download link generated. Tested for single and multiple selections of requsted logs. [EDIT] Proposed for dedicated issue, post Preliminary python 3.6 port. |
|
Confirming @FroggyFlox side-channel chat config-backup issue re port: System - Config backups - Proposed for dedicated issue. [EDIT] post Preliminary python 3.6 port. Noting a PyCharm editor warning on: src/rockstor/storageadmin/models/config_backup.py |
Includes prior TODO re ascii to utf-8 on SMART custom options.
Custom options autodev related byte assertion.
|
As this draft PR was a proof of concept and has got us to a mostly working Python 3.6 base it is proposed that I now move what we have to a non draft status after some squashing etc. We can then spin off, in testing dedicated issues. Most of which are likely type related. We also have the following test results to approach with likely a large amount of overlap with our proposed spin-off issues here: Ran 252 tests in 20.288s FAILED (failures=12, errors=6) |
|
Closing as superseded by: |
As part of our move to Python 3 we must first establish our new python 3 dependencies via Poetry. And resolve all issues re dependencies from there on.
In this Draft work-in-progress the base OS used in Leap 15.4.
See issue #2564 for context
Deleting poetry.lock and recreating via:
with the changes within this draft PR for discussion.
And then we see our first failure during the subsequent install/build of our stated dependencies:
As expected and detailed in our ongoing issue #1877
comment: #1877 (comment)
We have a blocker of gevent!