Update Django within current Python 2 constrains #2254 #2365

phillxnet · 2022-04-08T14:19:27Z

With reference to "move to python 3" GitHub #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.

See: #2254
For development notes.

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\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 Replace orphaned django-ztask with Huey #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. 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.
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.

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 · 2022-04-08T15:01:27Z

There remains a build error in this pr which looks to have arisen since I last worked on this transition.

Installing collectstatic.
/opt/rockstor-dev/eggs/psycopg2-2.7.4-py2.7-linux-x86_64.egg/psycopg2/__init__.py:144: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>.
  """)
Traceback (most recent call last):
  File "/opt/rockstor-dev/bin/django", line 36, in <module>
    sys.exit(djangorecipe.binscripts.manage('rockstor.settings'))
  File "/opt/rockstor-dev/eggs/djangorecipe-2.2.1-py2.7.egg/djangorecipe/binscripts.py", line 9, in manage
    management.execute_from_command_line(sys.argv)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/core/management/__init__.py", line 364, in execute_from_command_line
    utility.execute()
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/core/management/__init__.py", line 338, in execute
    django.setup()
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/__init__.py", line 27, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/apps/registry.py", line 108, in populate
    app_config.import_models()
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/apps/config.py", line 202, in import_models
    self.models_module = import_module(models_module_name)
  File "/usr/lib64/python2.7/importlib/__init__.py", line 37, in import_module
    __import__(name)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/models/__init__.py", line 43, in <module>
    from oauth_app import OauthApp  # noqa E501
  File "/opt/rockstor-dev/src/rockstor/storageadmin/models/oauth_app.py", line 20, in <module>
    from oauth2_provider.models import Application
  File "/opt/rockstor-dev/eggs/django_oauth_toolkit-1.1.2-py2.7.egg/oauth2_provider/models.py", line 15, in <module>
    from .generators import generate_client_id, generate_client_secret
  File "/opt/rockstor-dev/eggs/django_oauth_toolkit-1.1.2-py2.7.egg/oauth2_provider/generators.py", line 3, in <module>
    from oauthlib.common import generate_client_id as oauthlib_generate_client_id
  File "/opt/rockstor-dev/eggs/oauthlib-3.2.0-py2.7.egg/oauthlib/common.py", line 13, in <module>
    import urllib.parse as urlparse
ImportError: No module named parse

I am currently looking into this issue as I would like to maintain at least build capability prior to merging into testing.

FroggyFlox · 2022-04-08T15:11:02Z

@phillxnet ... I was just looking at it... Could the following be a clue?
oauthlib/oauthlib@b41d480

We might need to pin oauthlib version as well... although I'm not sure which module is listing it as a dependency.

phillxnet · 2022-04-08T15:15:51Z

@FroggyFlox Yes, this is what I was thinking. It's been a while since I've visited this code. I'll have a loot at additional pinning as it does very much look that way. Once sorted I'll push another commit to this branch as it's something that's changed since I was here last.

From earlier in the build:

Getting distribution for 'oauthlib>=2.0.3'.
  File "build/bdist.linux-x86_64/egg/oauthlib/oauth1/rfc5849/signature.py", line 55
    http_method: str,
               ^
SyntaxError: invalid syntax
zip_safe flag not set; analyzing archive contents...
  File "/opt/rockstor-dev/eggs/tmpRYbyUH/oauthlib-3.2.0-py2.7.egg/oauthlib/oauth1/rfc5849/signature.py", line 55
    http_method: str,
               ^
SyntaxError: invalid syntax
Got oauthlib 3.2.0.

FroggyFlox · 2022-04-08T15:51:17Z

Sounds good!
If I understand correctly, it seems that django-oauth_toolkit-1.1.2 has no "upper limit" on the oauth version so that might be why it imports the latest version it can:
https://github.com/jazzband/django-oauth-toolkit/blob/096ed111dc36d59c3d0281a249b596c180d220ec/setup.cfg#L30-L34

install_requires =
	django >= 1.11
	oauthlib >= 2.0.3
	requests >= 2.13.0

Our django-ouath-toolkit 1.1.2 has only >= 2.0.3 requirement. However as of oauthlib 3.1.1 (2021-05-31) Python 2.7 compatibility was dropped. Pinning to 3.1.0 for now as last known Python 2.7 compatible while also having Python 3.7 compatibility. Thanks to FroggyFlox on GitHub for some assistance in this matter.

phillxnet · 2022-04-08T17:49:30Z

From oauthlib changelog we have in version 3.1.1 (2021-05-31):
https://github.com/oauthlib/oauthlib/blob/master/CHANGELOG.rst#311-2021-05-31
734: python2 code removal

So: oauthlib 3.1.0 (2019-08-06) looks to be the last to support Python 2.7
https://github.com/oauthlib/oauthlib/blob/master/CHANGELOG.rst#310-2019-08-06
and Python 3.7 was added in 3.0.0:
https://github.com/oauthlib/oauthlib/blob/master/CHANGELOG.rst#300-2019-01-01

phillxnet · 2022-04-08T18:17:10Z

We are now at:

Ran 208 tests in 15.300s
FAILED (failures=5, errors=10)

which is up from when this code was last visited: #2254 (comment)

Ran 207 tests in 15.257s
FAILED (failures=4, errors=10)

However we have had some test changes in the interim.

EDIT: it has now been established that the extra test failure was down to a 'true' to True silent conversion.
Fixed in subsequent spin-off pr #2367. See below the the beginning of this spin-off issue/pr set.

New problem

We also appear to have another issue likely related to newer network code and it's interaction with the now updated Django:
Upon first Web-UI login we get a:

Houston, we've had a problem.
[u"'true' value must be either True or False."]

[08/Apr/2022 19:02:42] DEBUG [storageadmin.views.network:82] The function _update_or_create_ctype has been called
[08/Apr/2022 19:02:42] ERROR [storageadmin.util:45] Exception: [u"'true' value must be either True or False."]
Traceback (most recent call last):
  File "/opt/rockstor-dev/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception
    yield
  File "/opt/rockstor-dev/src/rockstor/storageadmin/views/network.py", line 600, in post
    self._refresh_connections()
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/utils/decorators.py", line 185, in inner
    return func(*args, **kwargs)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/views/network.py", line 184, in _refresh_connections
    cls._update_or_create_ctype(nco, ctype, ctype_d)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/utils/decorators.py", line 185, in inner
    return func(*args, **kwargs)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/views/network.py", line 129, in _update_or_create_ctype
    connection=co, usercon=usercon, **config
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/query.py", line 394, in create
    obj.save(force_insert=True, using=self.db)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/base.py", line 808, in save
    force_update=force_update, update_fields=update_fields)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/base.py", line 838, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/base.py", line 924, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/base.py", line 963, in _do_insert
    using=using, raw=raw)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/query.py", line 1079, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/sql/compiler.py", line 1111, in execute_sql
    for sql, params in self.as_sql():
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/sql/compiler.py", line 1064, in as_sql
    for obj in self.query.objs
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/sql/compiler.py", line 1003, in prepare_value
    value = field.get_db_prep_save(value, connection=self.connection)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/fields/__init__.py", line 770, in get_db_prep_save
    prepared=False)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/fields/__init__.py", line 762, in get_db_prep_value
    value = self.get_prep_value(value)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/fields/__init__.py", line 1043, in get_prep_value
    return self.to_python(value)
  File "/opt/rockstor-dev/eggs/Django-1.11.29-py2.7.egg/django/db/models/fields/__init__.py", line 1036, in to_python
    params={'value': value},
ValidationError: [u"'true' value must be either True or False."]
[08/Apr/2022 19:02:42] DEBUG [storageadmin.util:46] Current Rockstor version: Unknown Version

@FroggyFlox @Hooverdan96 This does look to be a differing treatment of this more recent network code by the Django update within this pr. Having a little look now. It may well account for the larger than expected test result differences. But I do see lots of object saves already which was a think I had to do through the code in this pr for many of the other tests to pass.
I'l like to get this sorted before merging this pull request as I remember login was a problem area and want to make sure this at least is working again. Curios that the error seems to suggest a string value i.e. unicode "true" being saved to a boolean model/db field within _update_or_create_ctype().

In initial bit of debug:

[08/Apr/2022 19:24:56] DEBUG [storageadmin.views.network:82] The function _update_or_create_ctype has been called
[08/Apr/2022 19:24:56] DEBUG [storageadmin.views.network:83] co=NetworkConnection object, ctype=bridge, config={'aux_address': None, 'subnet': u'172.17.0.0/16', 'internal': False, 'host_binding': u'0.0.0.0', 'docker_name': 'docker0', 'icc': u'true', 'ip_masquerade': False, 'dgateway': u'172.17.0.1', 'ip_range': None}
[08/Apr/2022 19:24:56] ERROR [storageadmin.util:45] Exception: [u"'true' value must be either True or False."]

The initial suspect here is the 'icc' field transitioned via **config re it's model entry in src/rockstor/storageadmin/models/network_interface.py

icc = models.BooleanField(default=False)

FroggyFlox · 2022-04-08T18:58:50Z

@phillxnet ... that's a bit I wrote for the implementation of rocknets. It's curious that it's a string, indeed, but that seems to be my fault.
We'll probably need to dig in on the output of the docker call there.

FroggyFlox · 2022-04-08T19:24:12Z

the icc key is fetched by the following:

rockstor-core/src/rockstor/system/network.py

Lines 234 to 237 in eeb0084

    
           if dtmap["Options"].get("com.docker.network.bridge.enable_icc"): 
        
               tmap[tmap["ctype"]]["icc"] = dtmap["Options"][ 
        
                   "com.docker.network.bridge.enable_icc" 
        
               ]

This dtmap dict is more or less the output of docker network inspect bridge in this case. In this get_con_config() call, it is (as obtained using the Pycharm python console:

{u'Ingress': False, u'Name': u'bridge', u'Created': u'2022-04-08T15:00:13.833568712-04:00', u'EnableIPv6': False,
 u'Labels': {}, u'Driver': u'bridge', u'Attachable': False, u'ConfigOnly': False, u'Internal': False,
 u'ConfigFrom': {u'Network': u''},
 u'Options': {u'com.docker.network.bridge.name': u'docker0', u'com.docker.network.bridge.default_bridge': u'true',
              u'com.docker.network.bridge.enable_ip_masquerade': u'true', u'com.docker.network.driver.mtu': u'1500',
              u'com.docker.network.bridge.host_binding_ipv4': u'0.0.0.0',
              u'com.docker.network.bridge.enable_icc': u'true'},
 u'IPAM': {u'Config': [{u'Subnet': u'172.17.0.0/16', u'Gateway': u'172.17.0.1'}], u'Driver': u'default',
           u'Options': None}, u'Scope': u'local',
 u'Id': u'e4691c34b67c4a98332f1e484f6d36cdae00ba4e3ab48f8286a0004d4c6098ee', u'Containers': {}}

So it seems it's coming from there as it is returned as a string and not a boolean as the other True/False values outside of the "Options" key.

I see two options there to fix this:

transform this into boolean (hacky but we could do some sort of if 'true': True else False
change this model's field as a text input rather than BooleanField, but that might require some changes where that is used.

What do you think? I feel I'm creeping outside the scope of this PR, though, so I can create an issue if you'd like.

FroggyFlox · 2022-04-08T19:29:42Z

Note that the ip_masquerade field should also be in the same situation if I'm correct:

rockstor-core/src/rockstor/system/network.py

Lines 239 to 244 in eeb0084

    
           if dtmap["Options"].get( 
        
               "com.docker.network.bridge.ip_masquerade" 
        
           ): 
        
               tmap[tmap["ctype"]]["ip_masquerade"] = dtmap["Options"][ 
        
                   "com.docker.network.bridge.ip_masquerade" 
        
               ]

phillxnet · 2022-04-08T19:33:03Z

@FroggyFlox Agreed. I'm just having a look now. I suspect this is a 'thing' where we got invisible string 'true' to boolean True conversion that has since been tighten-up as I've not seen this before here. I've been creeping up on it as it goes.

Re:

What do you think? I feel I'm creeping outside the scope of this PR, though, so I can create an issue if you'd like.

It may not be actually as it may be an artifact of the Django db population. Either way we can put it in a commit of it's own so if need be it could be cherry picked for stable if it comes up.

transform this into boolean (hacky but we could do some sort of if 'true': True else False

I much prefer this as we are already doing a tone of pre-processing in order to preset sanity and simplicity from the models to everything else. I'm almost there on this one I think. I'll post my findings shortly.

phillxnet · 2022-04-08T19:42:55Z

OK, we have our culprit as you suspected:

[08/Apr/2022 20:35:54] DEBUG [system.network:238] dtmap['Options']={u'com.docker.network.bridge.name': u'docker0', u'com.docker.network.bridge.default_bridge': u'true', u'com.docker.network.bridge.enable_ip_masquerade': u'true', u'com.docker.network.driver.mtu': u'1500', u'com.docker.network.bridge.host_binding_ipv4': u'0.0.0.0', u'com.docker.network.bridge.enable_icc': u'true'}
[08/Apr/2022 20:35:54] DEBUG [storageadmin.views.network:176] 'ctype' was found in config
[08/Apr/2022 20:35:54] DEBUG [storageadmin.views.network:179] we have extracted a ctype_d={'aux_address': None, 'subnet': u'172.17.0.0/16', 'internal': False, 'host_binding': u'0.0.0.0', 'docker_name': 'docker0', 'icc': u'true', 'ip_masquerade': False, 'dgateway': u'172.17.0.1', 'ip_range': None}
[08/Apr/2022 20:35:54] DEBUG [storageadmin.views.network:82] The function _update_or_create_ctype has been called
[08/Apr/2022 20:35:54] DEBUG [storageadmin.views.network:83] co=NetworkConnection object, ctype=bridge, config={'aux_address': None, 'subnet': u'172.17.0.0/16', 'internal': False, 'host_binding': u'0.0.0.0', 'docker_name': 'docker0', 'icc': u'true', 'ip_masquerade': False, 'dgateway': u'172.17.0.1', 'ip_range': None}
[08/Apr/2022 20:35:54] ERROR [storageadmin.util:45] Exception: [u"'true' value must be either True or False."]

Note also that as you say we have the same issue with "ip_masquerade", but we also have it with:

u'com.docker.network.bridge.default_bridge': u'true'

FroggyFlox · 2022-04-08T19:49:56Z

Thanks for the confirmation!

u'com.docker.network.bridge.default_bridge': u'true'

I believe we do not use that "option" so it's not a problem (to the best of my knowledge); I could misremember, though.

phillxnet · 2022-04-08T19:54:54Z

@FroggyFlox Funny you should say that. I was just looking for it and noticed it's lack of existence. We can always pop in a comment for future folk in case we end up parsing default_bridge as well. I'll sleep on this and hopefully fix tomorrow. I may end up popping this in an issue/pr set of it's own as it may have further ramifications so would be easier if we had a dedicated issue to reference. But I'd first like to prove the fix allows basic function for this pr before merging this and splitting the network fix out to it's own pr built on the then newly updated testing.

That is the network stuff here I think should go only to testing. It's too deep to go to master I think.

FroggyFlox · 2022-04-08T19:56:33Z

Sounds good!
Thanks a lot for the massive work there!

FroggyFlox · 2022-04-08T19:58:06Z

That is the network stuff here I think should go only to testing. It's too deep to go to master I think.

Agreed... especially given it doesn't seem to cause a problem in the Django version of the master branch anyway.

phillxnet · 2022-04-09T15:53:25Z

@FroggyFlox & @Hooverdan96
Having now sorted the last known pinning requirement (broken build) via an additional commit, and spun out the network 'true' to Boolean issue in #2366, for which I have a fix lined up which goes on top of this pull request. I'm going to merge this into testing so we can more easily engage with the numerous failures that still exist down to the Django update and related library updates. I have update the meta issue for this new testing branch/channel with some of the fields of failures still remaining:

WARNING -- EARLY TESTING CHANNEL -- KNOWN BROKEN (DEVELOPERS ONLY) #2362

As we go along we can link the specific GitHub issues and pull request fixes to that overall warning issue and keep that in play until we once again reach a mostly functional system. The we can begin on the Python 3 move hopefully.

…kstor#2384 As a result of major Django changes enacted in 'Update Django within current Python 2 constrains rockstor#2254' rockstor#2365, and following on from the fixture re-enablement spin-off of the former: '(t) Test fixtures non functional rockstor#2382' rockstor#2383 we have a test wide requirement to update or remove our now mostly outdated and newly re-enabled fixtures. Our prior work-around for the previously dysfunctional fixtures was extensive in-test db object mocking. This has become unworkable and incompatible with our newer Django. ## Includes - remove now outdated trailing '/' in task post calls rockstor#2384 Thanks to FroggyFlox for pointers: in GitHub issue rockstor#2370 we recently removed the trailing slash for post calls on tasks. Adjust task unit tests accordingly. - minor existing fixture update to be compatible with newer auth arrangement via independent fixture. - del redundant/legacy fixture/inheritance - multiple tests. - Addition on now independent test_api.json auth feature. - Black formatting. - del redundant/legacy fixture/inheritance - test_pool_balance. - Remove redundant fixtures and their references. - Addition on now independent test_api.json auth feature. - Add test run command. - Addition instructions on fixture content and creation. - Removing numerous now incompatible db object mocking as we now have working fixtures. - Improve test_api.py re setUpClass/tearDownClass supers rockstor#2382 Thanks to FroggyFlox on Github for follow-up work in this area. - Restore previously remarked out, existing snapshot delete test. - move smart run_test mock down to system - test_disk_smart.py. Thanks to FroggyFlox on GitHub. Our initial mock was failing, covered up by as yet only local testing. Moving the mock to the source of its definition resolves a failed mock. - Reviving a number of previously unworkable tests. - Improving test group names for clarity. - Test re-ordering for clarity. - Breaking out more tests into their own sets rather than repeat for all raid levels. - Improved/simplified mocking. - add todo, debug logging, and comments to pool.py concerning issue rockstor#2384

…kstor#2384 As a result of major Django changes enacted in 'Update Django within current Python 2 constrains rockstor#2254' rockstor#2365, and following on from the fixture re-enablement spin-off of the former: '(t) Test fixtures non functional rockstor#2382' rockstor#2383 we have a test wide requirement to update or remove our now mostly outdated and newly re-enabled fixtures. Our prior work-around for the previously dysfunctional fixtures was extensive in-test db object mocking. This has become unworkable and incompatible with our newer Django. ## Includes - remove now outdated trailing '/' in task post calls rockstor#2384 Thanks to FroggyFlox for pointers: in GitHub issue rockstor#2370 we recently removed the trailing slash for post calls on tasks. Adjust task unit tests accordingly. - minor existing fixture update to be compatible with newer auth arrangement via independent fixture. - del redundant/legacy fixture/inheritance - multiple tests. - Addition on now independent test_api.json auth feature. - Black formatting. - del redundant/legacy fixture/inheritance - test_pool_balance. - Remove redundant fixtures and their references. - Addition on now independent test_api.json auth feature. - Add test run command. - Addition instructions on fixture content and creation. - Removing numerous now incompatible db object mocking as we now have working fixtures. - Improve test_api.py re setUpClass/tearDownClass supers rockstor#2382 Thanks to FroggyFlox on Github for follow-up work in this area. - Restore previously remarked out, existing snapshot delete test. - move smart run_test mock down to system - test_disk_smart.py. Thanks to FroggyFlox on GitHub. Our initial mock was failing, covered up by as yet only local testing. Moving the mock to the source of its definition resolves a failed mock. - Reviving a number of previously unworkable tests. - Improving test group names for clarity. - Test re-ordering for clarity. - Breaking out more tests into their own sets rather than repeat for all raid levels. - Improved/simplified mocking. - add todo, debug logging, and comments to pool.py concerning issue rockstor#2384 - Simplify mocking of pool_usage in line with recent tested code changes.

…kstor#2384 As a result of major Django changes enacted in 'Update Django within current Python 2 constrains rockstor#2254' rockstor#2365, and following on from the fixture re-enablement spin-off of the former: '(t) Test fixtures non functional rockstor#2382' rockstor#2383 we have a test wide requirement to update or remove our now mostly outdated and newly re-enabled fixtures. Our prior work-around for the previously dysfunctional fixtures was extensive in-test db object mocking. This has become unworkable and incompatible with our newer Django. ## Includes - remove now outdated trailing '/' in task post calls rockstor#2384 Thanks to FroggyFlox for pointers: in GitHub issue rockstor#2370 we recently removed the trailing slash for post calls on tasks. Adjust task unit tests accordingly. - minor existing fixture update to be compatible with newer auth arrangement via independent fixture. - del redundant/legacy fixture/inheritance - multiple tests. - Addition on now independent test_api.json auth feature. - Black formatting. - del redundant/legacy fixture/inheritance - test_pool_balance. - Remove redundant fixtures and their references. - Addition on now independent test_api.json auth feature. - Add test run command. - Addition instructions on fixture content and creation. - Removing numerous now incompatible db object mocking as we now have working fixtures. - Improve test_api.py re setUpClass/tearDownClass supers rockstor#2382 Thanks to FroggyFlox on Github for follow-up work in this area. - Restore previously remarked out, existing snapshot delete test. - move smart run_test mock down to system - test_disk_smart.py. Thanks to FroggyFlox on GitHub. Our initial mock was failing, covered up by as yet only local testing. Moving the mock to the source of its definition resolves a failed mock. - Reviving a number of previously unworkable tests. - Improving test group names for clarity. - Test re-ordering for clarity. - Breaking out more tests into their own sets rather than repeat for all raid levels. - Improved/simplified mocking. - add todo, debug logging, and comments to pool.py concerning issue rockstor#2384 - Simplify mocking of pool_usage in line with recent tested code changes. - re-awaken more raid migration tests. - resolving prior PoolBalance anomaly - savepoint for issue rockstor#2384 We have strongly suspected code malfunction regarding pool balance status reporting. Adding initial comments to define new tests to establish expected behaviour to be used in another spin-off issue/pr set. - improve mocking accuracy test_pools.py. Depends on issue-pr (rockstor#2390/rockstor#2391) to abstract our balance command generation to improve future mocking and testing capabilities.

…kstor#2384 As a result of major Django changes enacted in 'Update Django within current Python 2 constrains rockstor#2254' rockstor#2365, and following on from the fixture re-enablement spin-off of the former: '(t) Test fixtures non functional rockstor#2382' rockstor#2383 we have a test wide requirement to update or remove our now mostly outdated and newly re-enabled fixtures. Our prior work-around for the previously dysfunctional fixtures was extensive in-test db object mocking. This has become unworkable and incompatible with our newer Django. ## Includes - remove now outdated trailing '/' in task post calls. Thanks to FroggyFlox for pointers: in GitHub issue rockstor#2370 we recently removed the trailing slash for post calls on tasks. Adjust task unit tests accordingly. - minor existing fixture update to be compatible with newer auth arrangement via independent fixture. - del redundant/legacy fixture/inheritance - multiple tests. - Addition on now independent test_api.json auth feature. - Black formatting. - del redundant/legacy fixture/inheritance - test_pool_balance. - Remove redundant fixtures and their references. - Addition on now independent test_api.json auth feature. - Add test run command. - Addition instructions on fixture content and creation. - Removing numerous now incompatible db object mocking as we now have working fixtures. - Improve test_api.py re setUpClass/tearDownClass supers rockstor#2382 Thanks to FroggyFlox on Github for follow-up work in this area. - Restore previously remarked out, existing snapshot delete test. - move smart run_test mock down to system - test_disk_smart.py. Thanks to FroggyFlox on GitHub. Our initial mock was failing, covered up by as yet only local testing. Moving the mock to the source of its definition resolves a failed mock. - Reviving a number of previously unworkable tests. - Improving test group names for clarity. - Test re-ordering for clarity. - Breaking out more tests into their own sets rather than repeat for all raid levels. - Improved/simplified mocking. - add todo, debug logging, and comments to pool.py concerning issue - Simplify mocking of pool_usage in line with recent tested code changes. - re-awaken more raid migration tests. - resolving prior PoolBalance anomaly - savepoint for issue We have strongly suspected code malfunction regarding pool balance status reporting. Adding initial comments to define new tests to establish expected behaviour to be used in another spin-off issue/pr set. - improve mocking accuracy test_pools.py. Depends on issue-pr (rockstor#2390/rockstor#2391) to abstract our balance command generation to improve future mocking and testing capabilities. - Includes reordering and splitting out more tests - By modifying our tid entries for the fixture PoolBalance we can aid test failure diagnosis via existing tid logging. - separate pool scrub and pool balance fixtures. - Add balance status reporting for cli initiated balance operations rockstor#2393 -- Rework PoolBalance to be more robust and cli balance aware. -- Improve ordering robustness by using order_by("start_time") as our prior by "id" can more fragile over such things as db imports. - API http post on balance status fails serialisation rockstor#2396 -- When we have no balance record avoid serialization failure by passing through our empty response from _balance_status(). - extend post cli balance status to multiple pools We initially tested on our data pool (in fixture) with two existing balance records. Extend to use also the ROOT pool (no balance records in fixture) to test our behaviour when no balance history exists: i.e. first record creation etc. - HTTP_204_NO_CONTENT for POST balance status empty rockstor#2396 When we have no last record we indicate as such via 204. Our existing behaviour on POST balance status command is to return latest record. When there isn't one we inform our caller of this via NO_CONTENT. - (t) ensure we message on all new cli balance records rockstor#2393 - (t) account for HTTP_204 in test_pool_balance.py Propose code now returns 204 when no prior balance exists and we do a POST balance status command. Inform our tests of this recent proposed code change in variable success returns. - t) improve post_valid_status test_pool_balance.py rockstor#2384 Now includes asserting latest return and recent NO_CONTENT return behaviour when there is no prior balance record. - POC for Huey aware balance reporting.

phillxnet · 2022-10-20T15:22:37Z

N.B. there are currently unmet migration requirements added by this now merged in testing branch pr that will be addressed in the ongoing and previously referenced:
#2400
Where an additional db migration is also required but his time concerning a refinement to the "poolbalance" model.

Outstanding "ordering" additions for recent Django update and an additional "get_latest_by" respectively.

Rock-Ons using the `DContainerDevice` fail to load the installation wizard with an AssertionError due a missing description of the fields attribute now required post our Django update (see rockstor#2365). Add the missing fields attribute to this serializer.

We previously recreated a scheduled task of type snapshot literally from the config backup file. As share IDs are highly likely to differ between system installations, this resulted in scheduled tasks of type snapshot being re-created upon config backup restore against the wrong share. This commit fixes this by: - fetching the share name from the ID from the config backup file (source) - fetch the corresponding share ID in the new (target) system - update the restore API call accordingly This commit also update the API endpoint for scheduled tasks following rockstor#2365.

We previously recreated a scheduled task of type snapshot literally from the config backup file. As share IDs are highly likely to differ between system installations, this resulted in scheduled tasks of type snapshot being re-created upon config backup restore against the wrong share. This commit fixes this by: 1. fetching the share name from the ID in the backup file (source) 2. fetching the corresponding share ID in the new (target) system 3. updating the restore API call accordingly This commit also fixes some related issues by: - updating the API endpoint for scheduled tasks following rockstor#2365 - continuing restore process if a taskdef fails to be validated rockstor#2355 Moreover, the current `restore_scheduled_tasks()` logic does both the preparation/parsing of the API payload, and the API call. This makes it difficult to test the former. This commit thus refactors this part to split into: - parsing/preparation: `validate_task_definitions()` - API call: `restore_scheduled_tasks()` Accordingly two new unit tests are included.

This was referenced Apr 9, 2022

[t] 'true' value must be either True or False." in network.py #2366

Closed

WARNING -- EARLY TESTING CHANNEL -- KNOWN BROKEN (DEVELOPERS ONLY) #2362

Closed

phillxnet merged commit 7d493cb into rockstor:testing Apr 9, 2022

phillxnet deleted the 2254_Update_Django_within_current_Python_2_constrains branch April 9, 2022 16:53

phillxnet mentioned this pull request Apr 9, 2022

[t] 'true' value must be either True or False." in network.py #2366 #2367

Merged

This was referenced Apr 18, 2022

(t) System-Services, Storage-Replication-Send_Receive, and Tasks pages fail #2370

Closed

(t) Failing unit tests #2371

Closed

phillxnet mentioned this pull request Jun 11, 2022

Modernization of source code #2375

Closed

This was referenced Jun 12, 2022

(t) Oauth2_provider database migration failure upon reboot #2376

Closed

(t) Django management commands are failing to run #2377

Closed

This was referenced Jun 17, 2022

(t) request.DATA NotImplementedError #2379

Closed

Test fixtures non functional #2382

Closed

phillxnet mentioned this pull request Jun 22, 2022

(t) Refresh or remove as required all test fixtures #2384

Closed

phillxnet mentioned this pull request Oct 7, 2022

(t) update tests #2384 #2400

Merged

4 tasks

phillxnet added a commit to phillxnet/rockstor-core that referenced this pull request Oct 20, 2022

(t) db migrations for GitHub pr rockstor#2365 and issue rockstor#2384

4f5881b

Outstanding "ordering" additions for recent Django update and an additional "get_latest_by" respectively.

This was referenced Oct 23, 2022

(t) initrock fails to assert changes in Django launch file #2402

Closed

(t) resolve multiple leaf nodes in db migration graph #2404

Closed

phillxnet mentioned this pull request Nov 10, 2022

Move to embed and update more dependencies #2424

Closed

12 tasks

phillxnet mentioned this pull request Nov 20, 2022

Update Django within current Python 2 constrains #2254

Closed

FroggyFlox mentioned this pull request Jan 3, 2023

(t) Missing serializers fields description in RockOnDeviceSerializer #2457

Closed

FroggyFlox mentioned this pull request Feb 26, 2023

Backup/Restore references incorrect shares for Scheduled Tasks #2355

Closed

FroggyFlox mentioned this pull request Mar 3, 2023

Convert share ID during snapshot task restore #2355 #2507

Merged

phillxnet mentioned this pull request Oct 25, 2023

Update django-oauth-toolkit #2710

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Django within current Python 2 constrains #2254 #2365

Update Django within current Python 2 constrains #2254 #2365

phillxnet commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

phillxnet commented Apr 8, 2022 •

edited

Loading

FroggyFlox commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 8, 2022

phillxnet commented Apr 8, 2022 •

edited

Loading

FroggyFlox commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

phillxnet commented Apr 8, 2022

phillxnet commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 8, 2022 •

edited

Loading

FroggyFlox commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

phillxnet commented Apr 9, 2022 •

edited

Loading

phillxnet commented Oct 20, 2022

Update Django within current Python 2 constrains #2254 #2365

Update Django within current Python 2 constrains #2254 #2365

Conversation

phillxnet commented Apr 8, 2022 • edited Loading

Includes

phillxnet commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

phillxnet commented Apr 8, 2022 • edited Loading

FroggyFlox commented Apr 8, 2022 • edited Loading

phillxnet commented Apr 8, 2022

phillxnet commented Apr 8, 2022 • edited Loading

New problem

FroggyFlox commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

phillxnet commented Apr 8, 2022

phillxnet commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022 • edited Loading

phillxnet commented Apr 8, 2022 • edited Loading

FroggyFlox commented Apr 8, 2022

FroggyFlox commented Apr 8, 2022

phillxnet commented Apr 9, 2022 • edited Loading

phillxnet commented Oct 20, 2022

phillxnet commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 8, 2022 •

edited

Loading

FroggyFlox commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 8, 2022 •

edited

Loading

FroggyFlox commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 8, 2022 •

edited

Loading

phillxnet commented Apr 9, 2022 •

edited

Loading