Merge pull request #16548 from edx/jeskew/more_mgmt_cmd_conv_from_optparse

Move more mgmt commands from optparse to argparse.

common/djangoapps/student/management/commands/bulk_change_enrollment.py

@@ -6,9 +6,9 @@ from django.db import transaction
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 from six import text_type
-from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 
 from course_modes.models import CourseMode
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from student.models import CourseEnrollment
 from xmodule.modulestore.django import modulestore
 
@@ -16,7 +16,9 @@ logger = logging.getLogger(__name__)  # pylint: disable=invalid-name
 
 
 class Command(BaseCommand):
-    """Management command to change many user enrollments at once."""
+    """
+    Management command to change many user enrollments at once.
+    """
 
     help = """
     Change the enrollment status for all users enrolled in a

openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py

@@ -2,16 +2,14 @@
 Django Management Command:  Generate Course Structure
 Generates and stores course structure information for one or more courses.
 """
-
 import logging
-from optparse import make_option
 
 from django.core.management.base import BaseCommand
 from opaque_keys.edx.keys import CourseKey
-from xmodule.modulestore.django import modulestore
+from six import text_type
 
 from openedx.core.djangoapps.content.course_structures.tasks import update_course_structure
-
+from xmodule.modulestore.django import modulestore
 
 log = logging.getLogger(__name__)
 
@@ -20,15 +18,13 @@ class Command(BaseCommand):
     """
     Generates and stores course structure information for one or more courses.
     """
-    args = '<course_id course_id ...>'
     help = 'Generates and stores course structure for one or more courses.'
 
-    option_list = BaseCommand.option_list + (
-        make_option('--all',
-                    action='store_true',
-                    default=False,
-                    help='Generate structures for all courses.'),
-    )
+    def add_arguments(self, parser):
+        parser.add_argument('course_id', nargs='*')
+        parser.add_argument('--all',
+                            action='store_true',
+                            help='Generate structures for all courses.')
 
     def handle(self, *args, **options):
         """
@@ -37,7 +33,7 @@ class Command(BaseCommand):
         if options['all']:
             course_keys = [course.id for course in modulestore().get_courses()]
         else:
-            course_keys = [CourseKey.from_string(arg) for arg in args]
+            course_keys = [CourseKey.from_string(arg) for arg in options['course_id']]
 
         if not course_keys:
             log.fatal('No courses specified.')
@@ -52,9 +48,9 @@ class Command(BaseCommand):
                 # TODO Future improvement: Use .delay(), add return value to ResultSet, and wait for execution of
                 # all tasks using ResultSet.join(). I (clintonb) am opting not to make this improvement right now
                 # as I do not have time to test it fully.
-                update_course_structure.apply(args=[unicode(course_key)])
+                update_course_structure.apply(args=[text_type(course_key)])
             except Exception as ex:
                 log.exception('An error occurred while generating course structure for %s: %s',
-                              unicode(course_key), ex.message)
+                              text_type(course_key), ex.message)
 
         log.info('Finished generating course structures.')

openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py

@@ -19,22 +19,23 @@ When reports are generated, we need to handle:
 The command will always use the read replica database if one is configured.
 
 """
-import os.path
+from __future__ import unicode_literals
+
+import contextlib
 import csv
-import time
 import datetime
-import contextlib
 import logging
-import optparse
+import os.path
+import time
 
-from django.core.management.base import BaseCommand, CommandError
 from django.conf import settings
+from django.core.management.base import BaseCommand, CommandError
 from django.db import connections
 from django.utils import timezone
-
 from opaque_keys.edx.keys import CourseKey
-from xmodule.modulestore.django import modulestore
+from six import text_type
 
+from xmodule.modulestore.django import modulestore
 
 DEFAULT_CHUNK_SIZE = 10
 
@@ -46,20 +47,28 @@ def chunks(sequence, chunk_size):
 
 
 class Command(BaseCommand):
-    """Generate a list of email opt-in values for user enrollments. """
-
-    args = "<OUTPUT_FILENAME> <ORG_ALIASES> --courses=COURSE_ID_LIST --email-optin-chunk-size=CHUNK_SIZE"
+    """
+    Generate a list of email opt-in values for user enrollments.
+    """
     help = "Generate a list of email opt-in values for user enrollments."
-    option_list = BaseCommand.option_list + (
-        optparse.make_option('--courses ', action='store'),
-        optparse.make_option(
-            '--email-optin-chunk-size',
-            action='store',
-            type='int',
-            default=DEFAULT_CHUNK_SIZE,
-            dest='email_optin_chunk_size',
-            help='The number of courses to get opt-in information for in a single query.')
-    )
+
+    def add_arguments(self, parser):
+        parser.add_argument('file_path',
+                            metavar='OUTPUT_FILENAME',
+                            help='Path where to output the email opt-in list.')
+        parser.add_argument('org_list',
+                            nargs='+',
+                            metavar='ORG_ALIASES',
+                            help='List of orgs for which to retrieve email opt-in info.')
+        parser.add_argument('--courses',
+                            metavar='COURSE_ID_LIST',
+                            help='List of course IDs for which to retrieve email opt-in info.')
+        parser.add_argument('--email-optin-chunk-size',
+                            type=int,
+                            default=DEFAULT_CHUNK_SIZE,
+                            dest='email_optin_chunk_size',
+                            metavar='CHUNK_SIZE',
+                            help='Number of courses for which to get opt-in information in a single query.')
 
     # Fields output in the CSV
     OUTPUT_FIELD_NAMES = [
@@ -77,10 +86,11 @@ class Command(BaseCommand):
     QUERY_INTERVAL = 1000
 
     # Default datetime if the user has not set a preference
-    DEFAULT_DATETIME_STR = datetime.datetime(year=2014, month=12, day=1).isoformat(' ')
+    DEFAULT_DATETIME_STR = datetime.datetime(year=2014, month=12, day=1).isoformat(str(' '))
 
     def handle(self, *args, **options):
-        """Execute the command.
+        """
+        Execute the command.
 
         Arguments:
             file_path (str): Path to the output file.
@@ -92,9 +102,12 @@ class Command(BaseCommand):
 
         Raises:
             CommandError
-
         """
-        file_path, org_list = self._parse_args(args)
+        file_path = options['file_path']
+        org_list = options['org_list']
+
+        if os.path.exists(file_path):
+            raise CommandError("File already exists at '{path}'".format(path=file_path))
 
         # Retrieve all the courses for the org.
         # If we were given a specific list of courses to include,
@@ -116,15 +129,15 @@ class Command(BaseCommand):
         # If no courses are found, abort
         if not courses:
             raise CommandError(
-                u"No courses found for orgs: {orgs}".format(
+                "No courses found for orgs: {orgs}".format(
                     orgs=", ".join(org_list)
                 )
             )
 
         # Let the user know what's about to happen
         LOGGER.info(
-            u"Retrieving data for courses: {courses}".format(
-                courses=", ".join([unicode(course) for course in courses])
+            "Retrieving data for courses: {courses}".format(
+                courses=", ".join([text_type(course) for course in courses])
             )
         )
 
@@ -137,44 +150,17 @@ class Command(BaseCommand):
                     self._write_email_opt_in_prefs(file_handle, org_list, course_group)
 
         # Remind the user where the output file is
-        LOGGER.info(u"Output file: {file_path}".format(file_path=file_path))
-
-    def _parse_args(self, args):
-        """Check and parse arguments.
-
-        Validates that the right number of args were provided
-        and that the output file doesn't already exist.
-
-        Arguments:
-            args (list): List of arguments given at the command line.
-
-        Returns:
-            Tuple of (file_path, org_list)
-
-        Raises:
-            CommandError
-
-        """
-        if len(args) < 2:
-            raise CommandError(u"Usage: {args}".format(args=self.args))
-
-        file_path = args[0]
-        org_list = args[1:]
-
-        if os.path.exists(file_path):
-            raise CommandError("File already exists at '{path}'".format(path=file_path))
-
-        return file_path, org_list
+        LOGGER.info("Output file: {file_path}".format(file_path=file_path))
 
     def _get_courses_for_org(self, org_aliases):
-        """Retrieve all course keys for a particular org.
+        """
+        Retrieve all course keys for a particular org.
 
         Arguments:
             org_aliases (list): List of aliases for the org.
 
         Returns:
             List of `CourseKey`s
-
         """
         all_courses = modulestore().get_courses()
         orgs_lowercase = [org.lower() for org in org_aliases]
@@ -186,14 +172,17 @@ class Command(BaseCommand):
 
     @contextlib.contextmanager
     def _log_execution_time(self):
-        """Context manager for measuring execution time. """
+        """
+        Context manager for measuring execution time.
+        """
         start_time = time.time()
         yield
         execution_time = time.time() - start_time
-        LOGGER.info(u"Execution time: {time} seconds".format(time=execution_time))
+        LOGGER.info("Execution time: {time} seconds".format(time=execution_time))
 
     def _write_email_opt_in_prefs(self, file_handle, org_aliases, courses):
-        """Write email opt-in preferences to the output file.
+        """
+        Write email opt-in preferences to the output file.
 
         This will generate a CSV with one row for each enrollment.
         This means that the user's "opt in" preference will be specified
@@ -209,14 +198,13 @@ class Command(BaseCommand):
 
         Returns:
             None
-
         """
         writer = csv.DictWriter(file_handle, fieldnames=self.OUTPUT_FIELD_NAMES)
         writer.writeheader()
 
         cursor = self._db_cursor()
         query = (
-            u"""
+            """
             SELECT
                 user.`id` AS `user_id`,
                 user.`username` AS username,
@@ -281,17 +269,17 @@ class Command(BaseCommand):
             row_count += 1
 
         # Log the number of rows we processed
-        LOGGER.info(u"Retrieved {num_rows} records.".format(num_rows=row_count))
+        LOGGER.info("Retrieved {num_rows} records.".format(num_rows=row_count))
 
     def _iterate_results(self, cursor):
-        """Iterate through the results of a database query, fetching in chunks.
+        """
+        Iterate through the results of a database query, fetching in chunks.
 
         Arguments:
             cursor: The database cursor
 
         Yields:
             tuple of row values from the query
-
         """
         while True:
             rows = cursor.fetchmany(self.QUERY_INTERVAL)
@@ -301,11 +289,15 @@ class Command(BaseCommand):
                 yield row
 
     def _sql_list(self, values):
-        """Serialize a list of values for including in a SQL "IN" statement. """
-        return u",".join([u'"{}"'.format(val) for val in values])
+        """
+        Serialize a list of values for including in a SQL "IN" statement.
+        """
+        return ",".join(['"{}"'.format(val) for val in values])
 
     def _db_cursor(self):
-        """Return a database cursor to the read replica if one is available. """
+        """
+        Return a database cursor to the read replica if one is available.
+        """
         # Use the read replica if one has been configured
         db_alias = (
             'read_replica'

openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py

@@ -9,7 +9,9 @@ from nose.plugins.attrib import attr
 
 import ddt
 from django.contrib.auth.models import User
+from django.core.management import call_command
 from django.core.management.base import CommandError
+from six import text_type
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -214,7 +216,7 @@ class EmailOptInListTest(ModuleStoreTestCase):
         output = self._run_command(self.TEST_ORG, chunk_size=2)
         course_ids = [row['course_id'].strip().decode('utf-8') for row in output]
         for course in self.courses:
-            assert unicode(course.id) in course_ids
+            assert text_type(course.id) in course_ids
 
     # Choose numbers before and after the query interval boundary
     @ddt.data(2, 3, 4, 5, 6, 7, 8, 9)
@@ -261,10 +263,10 @@ class EmailOptInListTest(ModuleStoreTestCase):
     def test_not_enough_args(self, num_args):
         args = ["dummy"] * num_args
         expected_msg_regex = (
-            "^Usage: <OUTPUT_FILENAME> <ORG_ALIASES> --courses=COURSE_ID_LIST --email-optin-chunk-size=CHUNK_SIZE$"
+            "^Error: too few arguments$"
         )
         with self.assertRaisesRegexp(CommandError, expected_msg_regex):
-            email_opt_in_list.Command().handle(*args)
+            call_command('email_opt_in_list', *args)
 
     def test_file_already_exists(self):
         temp_file = tempfile.NamedTemporaryFile(delete=True)
@@ -273,7 +275,7 @@ class EmailOptInListTest(ModuleStoreTestCase):
             temp_file.close()
 
         with self.assertRaisesRegexp(CommandError, "^File already exists"):
-            email_opt_in_list.Command().handle(temp_file.name, self.TEST_ORG)
+            call_command('email_opt_in_list', temp_file.name, self.TEST_ORG)
 
     def test_no_user_profile(self):
         """
@@ -376,7 +378,7 @@ class EmailOptInListTest(ModuleStoreTestCase):
         output_path = os.path.join(temp_dir_path, self.OUTPUT_FILE_NAME)
         org_list = [org] + other_names
         if only_courses is not None:
-            only_courses = ",".join(unicode(course_id) for course_id in only_courses)
+            only_courses = ",".join(text_type(course_id) for course_id in only_courses)
 
         command = email_opt_in_list.Command()
 
@@ -388,7 +390,7 @@ class EmailOptInListTest(ModuleStoreTestCase):
         kwargs = {'courses': only_courses}
         if chunk_size:
             kwargs['email_optin_chunk_size'] = chunk_size
-        command.handle(output_path, *org_list, **kwargs)
+        call_command('email_opt_in_list', output_path, *org_list, **kwargs)
 
         # Retrieve the output from the file
         try:
@@ -442,8 +444,8 @@ class EmailOptInListTest(ModuleStoreTestCase):
                     if hasattr(user, 'profile')
                     else ''
                 ),
-                "course_id": unicode(course_id).encode('utf-8'),
-                "is_opted_in_for_email": unicode(opt_in_pref),
+                "course_id": text_type(course_id).encode('utf-8'),
+                "is_opted_in_for_email": text_type(opt_in_pref),
                 "preference_set_datetime": (
                     self._latest_pref_set_datetime(self.user)
                     if kwargs.get("expect_pref_datetime", True)