Skip to content
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

move to python 3 #1877

Closed
phillxnet opened this issue Jan 1, 2018 · 9 comments
Closed

move to python 3 #1877

phillxnet opened this issue Jan 1, 2018 · 9 comments

Comments

@phillxnet
Copy link
Member

phillxnet commented Jan 1, 2018

Because Rockstor makes extensive use of Django we are compelled to move to Python 3 as our current legacy python base prohibits us from adopting Django 2.x or later. As the first issue of 2018 it seems fitting to formally address this technical debt, an inevitability in any long running project.

It is suggested that we target Python 3.6, however our existing repositories already have 3.4 available (epel); as such 3.4 may serve as an initial first step and is supported by Django 2.0 but is not supported by Django 2.1 (see "What Python version can I use with Django?"):
https://docs.djangoproject.com/en/2.0/faq/install/#what-python-version-can-i-use-with-django

Relevant to this issue is the pending demise of Python 2.7:
https://pythonclock.org/

Potential resources:
http://python3porting.com/
http://python-future.org/compatible_idioms.html
https://docs.python.org/3.0/whatsnew/3.0.html
https://docs.python.org/3.6/howto/pyporting.html#learn-the-differences-between-python-2-3

Please feel free to comment / advise on this issue and provide any resources that might help with this transition.

@MFlyer
Copy link
Member

MFlyer commented Jan 2, 2018

Hi @phillxnet & @schakrava ,
my 2 cents on moving to Python 3 and Django 2 (just moved to Django 2 on my daily job before xmas :D ) : if we want to move to python 3 to have django 2, right now we can avoid it (main diffs: better url.py files with things like "/myurl/slug:myslug" instead of reg exps + every migration requiring on cascade!), if instead we want to move to python 3...to have python 3, well, I'm ok...although python 2 is still faster than python 3 eheh

Mirko

@phillxnet
Copy link
Member Author

If we target at least Python 3.5 then we could move directly to Django Channels 2 (from gevent), as it has a reliance on the asyncio library and native Python async support:

https://channels.readthedocs.io/en/latest/one-to-two.html#requirements

Also Python 3.5 is the minimum for Django 2.1.

@bheiskell
Copy link

Hey folks,

I'm looking at rockstor for personal use, and I noticed that the version of django rockstor is using has fallen out of support for security fixes. There are a couple of cves for django 1.8.16. I thought about filing a separate ticket, but I figure it's not likely you'd go to 1.11 given that python 2 is falling out of support soon, so I'm tacking on here.

@phillxnet
Copy link
Member Author

@bheiskell Thanks and Yes, we do have rather a technical debt situation going on. Slowly working through things but very much intend to get to this. Slightly lower priority as I at least consider Rockstor to be mostly deployed on private networks but yes we have some work to do but as always there is quite a lot of it. Please consider joining our forum over at:
https://forum.rockstor.com/
and thanks again for the addition here.

And given the Python move is 'non trivial' we may just move to at least 1.11.18 in the mean time. We have quite a few moving parts as it goes so all depends on syncing them up where and when we can.

@phillxnet
Copy link
Member Author

Adding link to the python 2to3 library docs:
https://docs.python.org/2/library/2to3.html#to3-reference

@phillxnet
Copy link
Member Author

pip install caniusepython3
python3 setup.py caniusepython3
running caniusepython3
Finding and checking dependencies ...

You need 2 projects to transition to Python 3.
Of those 2 projects, 2 have no direct dependencies blocking their transition:

  django-ztask
  gevent-websocket

Linking to the following closed issues for context:

#1220 (early pinning and suggested replacement)
#1312
#1996
#1510

And from 1 month ago we have unknown compatibility with Python > 3.5 in gevent-websocket:
https://gitlab.com/noppo/gevent-websocket/-/commit/9b87b850f50bdce99561a95c561c01395a87df7c

@Conan-Kudo
Copy link

For what it's worth, with CentOS 7, CentOS 8 (with centosplus or elrepo kernels + btrfs-progs COPR), and openSUSE Leap 15.2, Python 3.6 is available and the default Python 3 version. Django 2.2 is available in both EPEL 8 and Leap 15.2, though nobody has packaged it for EPEL 7 today.

At this point, I think it'd be pretty safe to say you could move to Django 2 with Python 3.6+.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 8, 2022
With reference to "move to python 3" GitHub rockstor#1877 it is proposed that we
begin our Django update 'train' from our just prior to LTS version of
(1.8.16) to the last Python 2 supported version of (1.11.29). These
changes are intended to address the majority of updates required prior
to our Python 2 -> 3 move as newer versions of Django than (1.11.29)
will have to wait until our base code is transitioned to Python 3.

N.B. these changes are not sufficient for continued function across the
board but are intended as a stop-off point to encourage further
developer interest and help to promote co-operation in this address of
our existing technical dept.

## Includes
- Remove defunct patterns module: deprecated (1.8) removed (1.10).
- Url updates.
-- Move to using plain list for urlpatters construct.
-- Update appliances url to list format.
-- Remove defunct patterns use on storageadmin/smart_manager urls.
-- Remove defunct leading empty string in urlpatterns lists.
-- Move remaining urlpattern list elements to url() type.
Django 2.0 introduces path() to remove/replace regex requirement, with
re-path() as a direct drop-in replacement. But we are not there yet.
-- Normalise on url formatting, at least for now pre path() re_path().
Moved all root api page url definitions to top url file. If only one url
is then left in any include, remove that include. This allows for common
leading "/" prior to include without breaking the top api call, and
reduces the number of files to view in order to manage api reformatting.
-- Remove redundant leading "/" from all url elements. This tends to
numerous (urls.W002) warnings.
-- add now missing trailing '/' to include sub url lists.
These now replace the prior leading '/' within each include. But they
were removed due to the following warnings:
""?: (urls.W002) Your URL pattern '^/(?P<did>\d+)$' has a regex
beginning with a '/'."
"If this pattern is targeted in an include(), ensure the include()
pattern has a trailing '/'."
- Update Django requires/version specification.
- Move db_router.py allow_migrate to post 1.10 format. Our prior syntax
was deprecated in 1.8 and removed in 1.10.
- Upgrade django-rest-framework - 3.1.1 to 3.9.3 for Django compat. This
is the last version of django-rest-framework to support Python 2
- Upgrade django-oauth-toolkit and it's dependency 'Requests'.
- Requests in turn required a chardet update. Moved to latest and last
to support Python 2.7.
- Update import for newer django-oauth-toolkit.
- Highlight/todo django-ztask to Huey transition points rockstor#2276. These
have now been merged out by the subsequent Huey transition work.
- Django migrate --list deprecated (1.8), use showmigrations --list.
- Update to last release of djangorecipe from 1.9. Includes removal of
now redundant/deprecated "projectegg" option.
- Move to render() from removed render_to_response() in home.py.
- Remove now defunct TEMPLATE_* settings - set TEMPLATES equivalent.
- Apply remaining non-hard-coded oauth2_provider migrations.
Our prior move to Django 1.8 incorporated all migrations to that date,
including those for our prior django-oauth-toolkit == 0.9.0. This ended
up causing a column/field conflict when applying the full migrations
post updating to django-oauth-toolkit == 1.1.2. Resolve by fake applying
those we already have pre-applied.
- Update paths in django-hack for updated eggs.
- Update django-hack template to account for djangorecipe updates.
Djangorecipe no longer contains the manage unit so we revert to using a
mashup of path pre-definition and an upstream python 2 template for
django command line management.
- Update test-settings.conf.in re Django update - TEMPLATE settings.
Includes some outstanding huey migrations that were missed in the
django-ztask to huey move. Only affects the testing environment.
- Initial modification to serializers.py - djangorestframework update.
As from djangorestframework error we have:
"Creating a ModelSerializer without either the 'fields' attribute or the
'exclude' attribute has been deprecated since 3.3.0, and is now
disallowed."
Add required fields param serializers.py - djangorestframework update.
- Fix Django 1.11.29 errors re insufficient Disk Obj save() calls.
From ""./bin/test -v 3 -p test_commands.py" we previously received:
"ValueError: <Disk: Disk object> instance isn't saved. Use bulk=False or
save the object first."
in subtests of: test_bootstrap_command, and test_refresh_disk_state.
Both subtests related to _update_disk_state()
N.B. we may now over save in the earlier stages of _update_disk_state().
- Fix newer Django-rest-framwork warning re unordered object list.
test_get (storageadmin.tests.test_appliances.AppliancesTests):
"UnorderedObjectListWarning: Pagination may yield inconsistent results
with an unordered object_list:
<class 'storageadmin.models.appliance.Appliance'> QuerySet.
- Fix another disk instance isn't saved test failure.
test_compression (storageadmin.tests.test_pools.PoolTests):
ValueError: <Disk: Disk object> instance isn\'t saved. Use bulk=False or
save the object first.\n']
In this case we already proceed to save each disk object in turn but
this newer version of Django (1.11.29) responded to bulk=False option
which in turn calls each associated objects save() method.
- Another Django-rest-framwork warning re unordered object list.
unordered object_list: <class 'storageadmin.models.update_subscription.
UpdateSubscription'> QuerySet.
paginator = self.django_paginator_class(queryset, page_size)
Both reported within supervisord_gunicorn_stderr.log
- Add default ordering to some more models rockstor#2254
Addresses more test provoked warnings re:
"UnorderedObjectListWarning: Pagination may yield inconsistent results
with an unordered object_list"
- Restore prior default of APPEND_SLASH = False.
It is suspected that a newer Django began enabling this option if not
found.
- Remove CommonMiddleware and APPEND_SLASH setting for now rockstor#2254
- Add TODO's to appliances.py re exception handling.
- Order pool and network device model outputs by default.
- Add development logging for ongoing concerns.
- Modify imports re Django parse_header and rest_framework media.
- Add follow=true and APIClient to test_appliances.py.
- In progress modifications to explore some ongoing test failures.
- Add auto generated migration file related to Django update.
It seems we have a new requirement in newer django to explicitly state
our disk attached manager config.
@phillxnet
Copy link
Member Author

rleap15-3:/opt/rockstor-dev # python3 setup.py caniusepython3
running caniusepython3
Finding and checking dependencies ...

You need 1 project to transition to Python 3.
Of that 1 project, 1 has no direct dependencies blocking its transition:

  gevent-websocket

@phillxnet
Copy link
Member Author

As of current testing channel for rockstor-core, rockstor-jslibs, & rockstor-rpmbuild, we have our first Py3.6 based Leap 15.4 & 15.5 build successes. A new testing channel development phase will be initiated by the resulting builds shortly.

Closing this issue as per all linked issues & pull requests. And the associated items in the same milestone as this issue.

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

No branches or pull requests

4 participants