Migrate to support both Python 2 and Python 3 (oppia#7177)

* Migrate to Python 3 * fix lint * complete scripts * Fix lint * convert more files * Migrate platform/ and jobs * convert storage/ * Convert controllers/ * migrate domain/ * convert extensions/ * migrate export/ * migrate core/tests * fix * fix * revert base.py * fix tests * fix * fix * fix * fix * fix * fix * remove unicode literals * fix * fix tests * fix test * fix test * fix * fix * fix * fix * fix * add more functions in python_utils * fix * change location of python_utils * fix * add codeowner * install yaml * add os * fix tests * fix lint * fix lint * increase time * remove unnecessary future paths * fix lint * fix * fix lint * fix * fix lint * fix lint * fix lint * fix lint * fix last test * fix_lint * increase time * fix lint * fix lint * fix lint * fix lint * fix lint * fix lint * remove path * fix lint * fix lint * fix lint * run lint checks in parallel * remove timeout * address comments * address comments * fix * remove relative import * fix * install future as third party * fix lint * remove python_utils from report * remove division operator * add basestring to python_utils * removr str * add msg * fix tests * remove object * fix * fix lint * fix tests * remove range * fix lint * add lint * remove zip * remove map * remove metaclass * lint * add test * fix lint * fix lint * fix * add lint * remove round * remove next * remove input * fix test * fix tests * fix linter * fix lint * fix * address comments * address comments * address comments * lint for iteritems * fix * fix test * fix * fix * fix * fix * fix * remove iter * remove iter * fix lint * fix linter * lint * remove list * remove values and keys * fix * fix * fix * fix * address comments * address comments * fix tests * fix * address comments * address comments * remove list * add issue * fix backend tests * fix lint * use iter * use issue in todo * fix backend tests * fix test * address comments * remove print * remove boilerplate * fix lint * fix lint * fix test * address comments * extra comment * add absolute import * increase coverage * remove lines * empty * fix * fix * fix lint * fix lint * test linter * address comments * address comments * address comments * fix * fix * address comments * fix
matthewc2003 · Aug 23, 2019 · 7aaead7 · 7aaead7
1 parent 69668de
commit 7aaead7
Show file tree

Hide file tree

Showing 424 changed files with 3,794 additions and 1,954 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -56,10 +56,13 @@ jobs:
       - run: sudo pip install pyyaml
       - run:
           name: Run lint tests
+          # All the python scripts should behave as modules. Files like the
+          # pre_commit_linter and third_party_size_check need to import other
+          # Python files and that is only possible if we treat it as a module.
           command: |
             bash scripts/install_third_party.sh
-            python scripts/third_party_size_check.py
-            python scripts/pre_commit_linter.py --path=. --verbose
+            python -m scripts.third_party_size_check
+            python -m scripts.pre_commit_linter --path=. --verbose
 
   typescript_tests:
     <<: *job_defaults
@@ -72,7 +75,7 @@ jobs:
           name: Run typescript tests
           command: |
             bash scripts/install_third_party.sh
-            python scripts/typescript_checks.py
+            python -m scripts.typescript_checks
 
   frontend_tests:
     <<: *job_defaults
@@ -81,6 +84,7 @@ jobs:
       - run: date +%F > date
       - restore_cache:
           <<: *restore_cache
+      - run: sudo pip install pyyaml
       - run:
           name: Run frontend tests
           command: |
@@ -99,6 +103,7 @@ jobs:
       - run: date +%F > date
       - restore_cache:
           <<: *restore_cache
+      - run: sudo pip install pyyaml
       - run:
           name: Run backend tests
           command: |

diff --git a/.coveragerc b/.coveragerc
@@ -7,3 +7,6 @@ omit =
     *_test.py
     *core/tests/*
     *__init__.py
+    # TODO(#7419): Remove python_utils from the list once the codebase is
+    # run under Python 3.
+    python_utils.py
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -364,6 +364,7 @@
 /core/templates/dev/head/domain/utilities/ @DubeySandeep
 /schema_utils*.py @seanlip @DubeySandeep
 /utils*.py @aks681
+/python_utils*.py @lilithxxx
 
 
 # Restricted pages.

diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -14,7 +14,7 @@
 - [ ] The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
 - [ ] The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
 - [ ] The linter/Karma presubmit checks have passed.
-  - These should run automatically, but if not, you can manually trigger them locally using `python scripts/pre_commit_linter.py` and `bash scripts/run_frontend_tests.sh`.
+  - These should run automatically, but if not, you can manually trigger them locally using `python -m scripts.pre_commit_linter` and `bash scripts/run_frontend_tests.sh`.
 - [ ] The PR is made from a branch that's **not** called "develop".
 - [ ] The PR has an appropriate "PROJECT: ..." label (Please add this label for the first-pass review of the PR).
 - [ ] The PR has an appropriate "CHANGELOG: ..." label (If you are unsure of which label to add, ask the reviewers for guidance).

diff --git a/appengine_config.py b/appengine_config.py
@@ -15,6 +15,7 @@
 # pylint: skip-file
 
 """Configuration for App Engine."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 import logging
 import os
@@ -91,6 +92,7 @@ def save(self):
     os.path.join(ROOT_PATH, 'third_party', 'beautifulsoup4-4.7.1'),
     os.path.join(ROOT_PATH, 'third_party', 'bleach-3.1.0'),
     os.path.join(ROOT_PATH, 'third_party', 'callbacks-0.3.0'),
+    os.path.join(ROOT_PATH, 'third_party', 'future-0.17.1'),
     os.path.join(ROOT_PATH, 'third_party', 'gae-cloud-storage-1.9.22.1'),
     os.path.join(ROOT_PATH, 'third_party', 'gae-mapreduce-1.9.22.0'),
     os.path.join(ROOT_PATH, 'third_party', 'gae-pipeline-1.9.22.1'),

diff --git a/constants.py b/constants.py
@@ -17,11 +17,14 @@
 # pylint: disable=invalid-name
 
 """Loads constants for backend use."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 import json
 import os
 import re
 
+import python_utils
+
 
 def parse_json_from_js(js_file):
     """Extracts JSON object from JS file."""
@@ -45,5 +48,5 @@ class Constants(dict):
     __getattr__ = dict.__getitem__
 
 
-with open(os.path.join('assets', 'constants.js'), 'r') as f:
+with python_utils.open_file(os.path.join('assets', 'constants.js'), 'r') as f:
     constants = Constants(parse_json_from_js(f))
diff --git a/constants_test.py b/constants_test.py
@@ -13,12 +13,14 @@
 # limitations under the License.
 
 """Tests for Constants object and cosntants.json file."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 import os
 
-import constants  # pylint: disable=relative-import
-from core.tests import test_utils  # pylint: disable=relative-import
-import feconf  # pylint: disable=relative-import
+import constants
+from core.tests import test_utils
+import feconf
+import python_utils
 
 
 class ConstantsTests(test_utils.GenericTestBase):
@@ -30,7 +32,8 @@ def test_constants_file_is_existing(self):
 
     def test_constants_file_contains_valid_json(self):
         """Test if the constants file is valid json file."""
-        with open(os.path.join('assets', 'constants.js'), 'r') as f:
+        with python_utils.open_file(
+            os.path.join('assets', 'constants.js'), 'r') as f:
             json = constants.parse_json_from_js(f)
             self.assertTrue(isinstance(json, dict))
             self.assertEqual(json['TESTING_CONSTANT'], 'test')
@@ -47,8 +50,9 @@ def test_constants_and_feconf_are_consistent(self):
 
     def test_all_comments_are_removed_from_json_text(self):
         """Tests if comments are removed from json text."""
-        with open(os.path.join(
-            feconf.TESTS_DATA_DIR, 'dummy_constants.js'), 'r') as f:
+        dummy_constants_filepath = os.path.join(
+            feconf.TESTS_DATA_DIR, 'dummy_constants.js')
+        with python_utils.open_file(dummy_constants_filepath, 'r') as f:
             actual_text_without_comments = constants.remove_comments(f.read())
             expected_text_without_comments = (
                 'var dummy_constants = {\n'

diff --git a/core/controllers/acl_decorators.py b/core/controllers/acl_decorators.py
@@ -15,9 +15,9 @@
 # limitations under the License.
 
 """Decorators to provide authorization across the site."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 import functools
-import urllib
 
 from core.controllers import base
 from core.domain import feedback_services
@@ -36,6 +36,7 @@
 from core.domain import user_services
 from core.platform import models
 import feconf
+import python_utils
 
 current_user_services = models.Registry.import_current_user_services()
 
@@ -2319,7 +2320,7 @@ def test_can_access(self, topic_name, **kwargs):
         Raises:
             PageNotFoundException: The given page cannot be found.
         """
-        topic_name = urllib.unquote_plus(topic_name)
+        topic_name = python_utils.url_unquote_plus(topic_name)
         topic = topic_fetchers.get_topic_by_name(topic_name)
 
         if topic is None:

diff --git a/core/controllers/acl_decorators_test.py b/core/controllers/acl_decorators_test.py
@@ -15,6 +15,7 @@
 # limitations under the License.
 
 """Tests for core.domain.acl_decorators."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 from core.controllers import acl_decorators
 from core.controllers import base

diff --git a/core/controllers/admin.py b/core/controllers/admin.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 """Controllers for the admin view."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 import logging
 import random
@@ -38,6 +39,7 @@
 from core.domain import user_services
 from core.platform import models
 import feconf
+import python_utils
 import utils
 
 current_user_services = models.Registry.import_current_user_services()
@@ -60,7 +62,7 @@ class AdminHandler(base.BaseHandler):
     @acl_decorators.can_access_admin_page
     def get(self):
         """Handles GET requests."""
-        demo_exploration_ids = feconf.DEMO_EXPLORATIONS.keys()
+        demo_exploration_ids = list(feconf.DEMO_EXPLORATIONS.keys())
 
         recent_job_data = jobs.get_data_for_recent_jobs()
         unfinished_job_data = jobs.get_data_for_unfinished_jobs()
@@ -107,8 +109,8 @@ def get(self):
             'config_properties': (
                 config_domain.Registry.get_config_property_schemas()),
             'continuous_computations_data': continuous_computations_data,
-            'demo_collections': sorted(feconf.DEMO_COLLECTIONS.iteritems()),
-            'demo_explorations': sorted(feconf.DEMO_EXPLORATIONS.iteritems()),
+            'demo_collections': sorted(feconf.DEMO_COLLECTIONS.items()),
+            'demo_explorations': sorted(feconf.DEMO_EXPLORATIONS.items()),
             'demo_exploration_ids': demo_exploration_ids,
             'human_readable_current_time': (
                 utils.get_human_readable_time_string(
@@ -167,7 +169,7 @@ def post(self):
                     'new_config_property_values')
                 logging.info('[ADMIN] %s saved config property values: %s' %
                              (self.user_id, new_config_property_values))
-                for (name, value) in new_config_property_values.iteritems():
+                for (name, value) in new_config_property_values.items():
                     config_services.set_property(self.user_id, name, value)
             elif self.payload.get('action') == 'revert_config_property':
                 config_property_id = self.payload.get('config_property_id')
@@ -208,7 +210,7 @@ def post(self):
                 recommendations_services.update_topic_similarities(data)
             self.render_json({})
         except Exception as e:
-            self.render_json({'error': unicode(e)})
+            self.render_json({'error': python_utils.STR(e)})
             raise
 
     def _reload_exploration(self, exploration_id):
@@ -225,9 +227,11 @@ def _reload_exploration(self, exploration_id):
             logging.info(
                 '[ADMIN] %s reloaded exploration %s' %
                 (self.user_id, exploration_id))
-            exp_services.load_demo(unicode(exploration_id))
+            exp_services.load_demo(python_utils.convert_to_bytes(
+                exploration_id))
             rights_manager.release_ownership_of_exploration(
-                user_services.get_system_user(), unicode(exploration_id))
+                user_services.get_system_user(), python_utils.convert_to_bytes(
+                    exploration_id))
         else:
             raise Exception('Cannot reload an exploration in production.')
 
@@ -245,9 +249,11 @@ def _reload_collection(self, collection_id):
             logging.info(
                 '[ADMIN] %s reloaded collection %s' %
                 (self.user_id, collection_id))
-            collection_services.load_demo(unicode(collection_id))
+            collection_services.load_demo(
+                python_utils.convert_to_bytes(collection_id))
             rights_manager.release_ownership_of_collection(
-                user_services.get_system_user(), unicode(collection_id))
+                user_services.get_system_user(), python_utils.convert_to_bytes(
+                    collection_id))
         else:
             raise Exception('Cannot reload a collection in production.')
 
@@ -274,7 +280,7 @@ def _generate_dummy_explorations(
                                'Elvish, language of "Lord of the Rings',
                                'The Science of Superheroes']
             exploration_ids_to_publish = []
-            for i in range(num_dummy_exps_to_generate):
+            for i in python_utils.RANGE(num_dummy_exps_to_generate):
                 title = random.choice(possible_titles)
                 category = random.choice(constants.SEARCH_DROPDOWN_CATEGORIES)
                 new_exploration_id = exp_fetchers.get_new_exploration_id()

diff --git a/core/controllers/admin_test.py b/core/controllers/admin_test.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 """Tests for the admin page."""
+from __future__ import absolute_import  # pylint: disable=import-only-modules
 
 import logging
 
@@ -37,7 +38,6 @@
 from core.tests import test_utils
 import feconf
 
-
 (exp_models, job_models,) = models.Registry.import_models(
     [models.NAMES.exploration, models.NAMES.job])