chore: Dead dependencies and code cleanup #13344

gavindsouza · 2021-05-26T11:41:53Z

Changes

Removed future, past utils' usages (includes unicode_literals, print_function, with_metaclass, cmp)
Removed code blocks for if PY2 and if not PY3, and other variations of it; and other compatibility code
Removed six usages (includes string_types, text_type, integer_types, urllib, StringIO, etc)
Used context managers at multiple places to ensure files are closed after reading them
Removed watchdog, future, bleach-whitelist packages from requirements.txt
Added bleach-allowlist as replacement for bleach-whitelist in requirements.txt
Escaped and used raw strings to handle invalid sequence errors and deprecation warnings
Converted strings to translatable for certain throws (as pointed out by semgrep)
Style fixes (as pointed out by sider), and code simplification
Dropped test_translation.py which has been completely commented out for ~7 years
Updated Jinja2 from 2.11.3 to 3.0.1
Updated PyJWT from 1.7.1 to 2.0.1

Check commit messages for more details

cmp was being used from past.builtins library since it was deprecated in PY2. It's hard to understand behaviour of their usages, so this is an attempt to replicate behaviour with simpler logic, making this more readable. Also, removed usages of iteritems and string_types, compatibility imports

* Remove six for PY2 compatability since our dependencies are not, PY2 is legacy. * Removed usages of utils from future/past libraries since they are deprecated. This includes 'from __future__ ...' and 'from past...' statements. * Removed compatibility imports for PY2, switched from six imports to standard library imports. * Removed utils code blocks that handle operations depending on PY2/3 versions. * Removed 'from __future__ ...' lines from templates/code generators * Used PY3 syntaxes in place of PY2 compatible blocks. eg: metaclass

* The library bleach-whitelist was deprecated and renamed to bleach-allowlist. * Updated the usages and requirements for the same.

While executing git commands in the shell via Frappe processes, use context managers to ensure files get closed after usage. This fixes the ResourceWarning errors due to unclosed files.

Watchdog isn't used by Frappe, and there wasn't any mechanism to access it directly either. By default, bench serve (or start) uses Werkzeug's watchdogreloader

This fixes the ResourceWarning errors due to unclosed files while utilizing the website router

Escaped when proven easier

coveralls · 2021-05-26T14:39:07Z

Coverage decreased (-0.08%) to 52.103% when pulling 2ad9d20 on gavindsouza:drop-py2-code into 099c2f0 on frappe:develop.

reported by sider: https://sider.review/gh/repos/1864194/pulls/13344?statuses#issues

* Remove trailing whitespaces in translatable strings * Sort imports appropriately

Simplify conditional block and str appending

gavindsouza · 2021-05-27T10:44:51Z

Found the following deprecation warnings in the CI tests that broke after the last merge. Will accommodate them in this PR.

/home/runner/frappe-bench/env/lib/python3.7/site-packages/jinja2/environment.py:1088: DeprecationWarning: 'soft_unicode' has been renamed to 'soft_str'. The old name will be removed in MarkupSafe 2.1.

DeprecationWarning: deprecated [__init__.py:6]

DeprecationWarning: It is strongly recommended that you pass in a value for the "algorithms" argument when calling decode(). This argument will be mandatory in a future version. [api_jws.py:145]

Edit: Updated Jinja2 and PyJWT to get rid of those.

Update from 2.11.3 to 3.0.1 Ref: https://jinja.palletsprojects.com/en/3.0.x/changes/

* Update pinned dep fromm 1.7.1 to 2.0.1 * Updated usages as per changelog ref: https://python.libhunt.com/pyjwt-changelog

revant · 2021-05-31T15:48:20Z

bench setup requirements solved previous error,

❯ bench --site test.localhost reinstall --yes --admin-password admin

Installing frappe...
Updating DocTypes for frappe        : [====                                    ] 12%Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/database/database.py", line 143, in sql
    self._cursor.execute(query)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/cursors.py", line 148, in execute
    result = self._query(query)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/cursors.py", line 310, in _query
    conn.query(q)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/connections.py", line 548, in query
    self._affected_rows = self._read_query_result(unbuffered=unbuffered)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/connections.py", line 775, in _read_query_result
    result.read()
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/connections.py", line 1156, in read
    first_packet = self.connection._read_packet()
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/connections.py", line 725, in _read_packet
    packet.raise_for_error()
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/protocol.py", line 221, in raise_for_error
    err.raise_mysql_exception(self._data)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/pymysql/err.py", line 143, in raise_mysql_exception
    raise errorclass(errno, errval)
pymysql.err.ProgrammingError: (1146, "Table '_fcadc07dbdbc21db.tabDocument Naming Rule' doesn't exist")

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/revant/.pyenv/versions/3.7.10/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/revant/.pyenv/versions/3.7.10/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/revant/frappe-bench/apps/frappe/frappe/utils/bench_helper.py", line 104, in <module>
    main()
  File "/home/revant/frappe-bench/apps/frappe/frappe/utils/bench_helper.py", line 18, in main
    click.Group(commands=commands)(prog_name='bench')
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/decorators.py", line 21, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/commands/__init__.py", line 26, in _func
    ret = f(frappe._dict(ctx.obj), *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/commands/site.py", line 148, in reinstall
    _reinstall(site, admin_password, mariadb_root_username, mariadb_root_password, yes, verbose=context.verbose)
  File "/home/revant/frappe-bench/apps/frappe/frappe/commands/site.py", line 171, in _reinstall
    admin_password=admin_password)
  File "/home/revant/frappe-bench/apps/frappe/frappe/installer.py", line 79, in _new_site
    install_app(app, verbose=verbose, set_as_patched=not source_sql)
  File "/home/revant/frappe-bench/apps/frappe/frappe/installer.py", line 160, in install_app
    sync_for(name, force=True, sync_everything=True, verbose=verbose, reset_permissions=True)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/sync.py", line 68, in sync_for
    reset_permissions=reset_permissions, for_sync=True)
  File "/home/revant/frappe-bench/apps/frappe/frappe/modules/import_file.py", line 67, in import_file_by_path
    ignore_version=ignore_version, reset_permissions=reset_permissions)
  File "/home/revant/frappe-bench/apps/frappe/frappe/modules/import_file.py", line 149, in import_doc
    doc.insert()
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 265, in insert
    self.run_post_save_methods()
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 991, in run_post_save_methods
    self.run_method("on_update")
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 856, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 1145, in composer
    return composed(self, method, *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 1128, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 850, in <lambda>
    fn = lambda self, *args, **kwargs: getattr(self, method)(*args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/doctype/doctype.py", line 300, in on_update
    make_module_and_roles(self)
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/doctype/doctype.py", line 1214, in make_module_and_roles
    r.insert()
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 230, in insert
    self.set_new_name(set_name=set_name, set_child_names=set_child_names)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 420, in set_new_name
    set_new_name(self)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/naming.py", line 41, in set_new_name
    set_naming_from_document_naming_rule(doc)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/naming.py", line 95, in set_naming_from_document_naming_rule
    dict(document_type=doc.doctype, disabled=0), order_by='priority desc', ignore_ddl=True):
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 1432, in get_all
    return get_list(doctype, *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 1405, in get_list
    return frappe.model.db_query.DatabaseQuery(doctype).execute(*args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/db_query.py", line 101, in execute
    result = self.build_and_run()
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/db_query.py", line 139, in build_and_run
    update=self.update, ignore_ddl=self.ignore_ddl)
  File "/home/revant/frappe-bench/apps/frappe/frappe/database/database.py", line 161, in sql
    if ignore_ddl and (self.is_missing_column(e) or self.is_missing_table(e) or self.cant_drop_field_or_key(e)):
AttributeError: 'MariaDBDatabase' object has no attribute 'is_missing_table'

reinstall again solved it.

I'm not able to successfully run tests locally

❯ bench --site test.localhost run-tests --module frappe.tests.test_oauth20
Updating Dashboard for frappe

test_invalid_login (frappe.tests.test_oauth20.TestOAuth20) (4.33s)
.EEFEEE
======================================================================
ERROR: test_login_using_authorization_code (frappe.tests.test_oauth20.TestOAuth20)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_oauth20.py", line 64, in test_login_using_authorization_code
    auth_code = query.get("code")[0]
TypeError: 'NoneType' object is not subscriptable

======================================================================
ERROR: test_login_using_authorization_code_with_pkce (frappe.tests.test_oauth20.TestOAuth20)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_oauth20.py", line 116, in test_login_using_authorization_code_with_pkce
    auth_code = query.get("code")[0]
TypeError: 'NoneType' object is not subscriptable

======================================================================
ERROR: test_openid_code_id_token (frappe.tests.test_oauth20.TestOAuth20)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_oauth20.py", line 282, in test_openid_code_id_token
    auth_code = query.get("code")[0]
TypeError: 'NoneType' object is not subscriptable

======================================================================
ERROR: test_resource_owner_password_credentials_grant (frappe.tests.test_oauth20.TestOAuth20)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_oauth20.py", line 216, in test_resource_owner_password_credentials_grant
    bearer_token = token_response.json()
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/requests/models.py", line 900, in json
    return complexjson.loads(self.text, **kwargs)
  File "/home/revant/.pyenv/versions/3.7.10/lib/python3.7/json/__init__.py", line 348, in loads
    return _default_decoder.decode(s)
  File "/home/revant/.pyenv/versions/3.7.10/lib/python3.7/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/home/revant/.pyenv/versions/3.7.10/lib/python3.7/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

======================================================================
ERROR: test_revoke_token (frappe.tests.test_oauth20.TestOAuth20)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_oauth20.py", line 166, in test_revoke_token
    auth_code = query.get("code")[0]
TypeError: 'NoneType' object is not subscriptable

======================================================================
FAIL: test_login_using_implicit_token (frappe.tests.test_oauth20.TestOAuth20)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_oauth20.py", line 249, in test_login_using_implicit_token
    self.assertTrue(response_dict.get("access_token"))
AssertionError: None is not true

----------------------------------------------------------------------
Ran 7 tests in 4.612s

FAILED (failures=1, errors=5)
❯ code sites/test.localhost/site_config.json
❯ bench --site test.localhost reinstall --yes --admin-password admin

Installing frappe...
Updating DocTypes for frappe        : [========================================] 100%
Updating country info               : [========================================] 100%
*** Scheduler is disabled ***
❯ bench --site test.localhost run-tests --module frappe.tests.test_api
Updating Dashboard for frappe
..FFFFFFF.F
======================================================================
FAIL: test_create_document (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 122, in test_create_document
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

======================================================================
FAIL: test_delete_document (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 143, in test_delete_document
    self.assertEqual(response.status_code, 202)
AssertionError: 403 != 202

======================================================================
FAIL: test_get_list (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 79, in test_get_list
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

======================================================================
FAIL: test_get_list_debug (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 106, in test_get_list_debug
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

======================================================================
FAIL: test_get_list_dict (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 93, in test_get_list_dict
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

======================================================================
FAIL: test_get_list_fields (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 114, in test_get_list_fields
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

======================================================================
FAIL: test_get_list_limit (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 86, in test_get_list_limit
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

======================================================================
FAIL: test_update_document (frappe.tests.test_api.TestResourceAPI)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/tests/test_api.py", line 135, in test_update_document
    self.assertEqual(response.status_code, 200)
AssertionError: 403 != 200

----------------------------------------------------------------------
Ran 11 tests in 4.406s

FAILED (failures=8)

…on_key (#13399) * fix: only allow keys generated by fernet in encrypt()/decrypt() * fix: sider and semgrep fixes

revant · 2021-06-03T07:13:40Z

I created a fresh develop bench and everything worked.

Sorry for wrong review and delayed reply.

gavindsouza added 11 commits May 26, 2021 13:25

chore: Replacing and updating bleach source list

d236a93

* The library bleach-whitelist was deprecated and renamed to bleach-allowlist. * Updated the usages and requirements for the same.

fix: Use context managers to stream data

1acb2dd

While executing git commands in the shell via Frappe processes, use context managers to ensure files get closed after usage. This fixes the ResourceWarning errors due to unclosed files.

fix: Use dict.items instead of six.iteritems

1e9b0cf

fix: Use raw string to avoid invalid escape sequence errors

feea2f3

chore: Drop unreachable code

bd6fcdd

chore: Drop watchdog dependency

3adb84e

Watchdog isn't used by Frappe, and there wasn't any mechanism to access it directly either. By default, bench serve (or start) uses Werkzeug's watchdogreloader

fix: Close file after read

131cfae

This fixes the ResourceWarning errors due to unclosed files while utilizing the website router

fix: Use raw string to avoid invalid sequence errors

f6b2159

Escaped when proven easier

chore: Drop future from requirements.txt

295d44c

gavindsouza mentioned this pull request May 26, 2021

Depreciation Warning again #13331

Closed

fix(typo): Let's try executing bound method instead

88ef41b

gavindsouza added 5 commits May 26, 2021 22:21

chore: Drop FileNotFoundError handling for PY2

eca3066

style: Fix E201, E202, E401, E701

793b563

reported by sider: https://sider.review/gh/repos/1864194/pulls/13344?statuses#issues

fix: Translate strings passed in frappe.throw

e2ee75e

* Remove trailing whitespaces in translatable strings * Sort imports appropriately

fix: Remove trailing whitespace in translatable str

6fb1412

Simplify conditional block and str appending

chore: Drop commented out test_translation file

96c03fd

gavindsouza marked this pull request as ready for review May 27, 2021 09:42

gavindsouza requested review from leela, prssanna, ruchamahabal, surajshetty3416 and a team as code owners May 27, 2021 09:42

gavindsouza mentioned this pull request May 27, 2021

fix: Add future in requirements.txt frappe/erpnext#25850

Merged

gavindsouza added the dont-merge label May 27, 2021

chore: Remove temporary warnings filter

72c2393

gavindsouza force-pushed the drop-py2-code branch from ee68a16 to 72c2393 Compare May 27, 2021 11:10

gavindsouza added 5 commits May 28, 2021 10:51

Merge branch 'develop' into drop-py2-code

4e75260

fix: Return address 'name' instead of 'parent' field

651a9ff

chore: Update Jinja2 dependency

9b4c191

Update from 2.11.3 to 3.0.1 Ref: https://jinja.palletsprojects.com/en/3.0.x/changes/

chore: Update PyJWT dependency

165ff8e

* Update pinned dep fromm 1.7.1 to 2.0.1 * Updated usages as per changelog ref: https://python.libhunt.com/pyjwt-changelog

Merge branch 'develop' of github.com:frappe/frappe into drop-py2-code

8558116

This comment has been minimized.

Sign in to view

gavindsouza referenced this pull request Jun 3, 2021

fix: Allow only use of Fernet generated key for using custom encrypti…

464e93a

…on_key (#13399) * fix: only allow keys generated by fernet in encrypt()/decrypt() * fix: sider and semgrep fixes

Merge branch 'develop' of github.com:frappe/frappe into drop-py2-code

2ad9d20

gavindsouza merged commit f36513d into frappe:develop Jun 3, 2021

centaur78 mentioned this pull request Jun 3, 2021

Depreciation warning for imp module and bleach_whitelist #13283

Closed

gavindsouza removed the dont-merge label Jun 4, 2021

ankush mentioned this pull request Jul 11, 2022

V14 migration guide / breaking changes #17458

Closed

51 tasks

This was referenced Jul 18, 2022

v14 Features & Enhancements surajshetty3416/frappe#25

Closed

List of V14 features & enhancements #17532

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Dead dependencies and code cleanup #13344

chore: Dead dependencies and code cleanup #13344

gavindsouza commented May 26, 2021 •

edited

coveralls commented May 26, 2021 •

edited

gavindsouza commented May 27, 2021 •

edited

This comment has been minimized.

revant commented May 31, 2021 •

edited

revant commented Jun 3, 2021

chore: Dead dependencies and code cleanup #13344

chore: Dead dependencies and code cleanup #13344

Conversation

gavindsouza commented May 26, 2021 • edited

Changes

coveralls commented May 26, 2021 • edited

gavindsouza commented May 27, 2021 • edited

This comment has been minimized.

revant commented May 31, 2021 • edited

revant commented Jun 3, 2021

gavindsouza commented May 26, 2021 •

edited

coveralls commented May 26, 2021 •

edited

gavindsouza commented May 27, 2021 •

edited

revant commented May 31, 2021 •

edited