Allow generator to send args to correct script.

fix yaml.load() to yaml.safe_load() as per deprecation warning. fix flake8 errors

Allow generator to send args to correct script.
fix yaml.load() to yaml.safe_load() as per deprecation warning. fix flake8 errors
Miguel Barão
1 parent 50eac3c3
Showing 8 changed files with 121 additions and 92 deletions Show diff stats
BUGS.md
README.md
perguntations/app.py
perguntations/factory.py
perguntations/initdb.py
perguntations/questions.py
perguntations/serve.py
perguntations/tools.py
@@ -8,7 +8,6 @@ e.g. retornar None quando nao ha alteracoes relativamente à última vez.
 ou usar push (websockets?)
 - pymips: nao pode executar syscalls do spim.
 - perguntas checkbox [right,wrong] com pelo menos uma opção correcta.
-- questions.py textarea has a abspath which does not make sense! why is it there? not working for perguntations, but seems to work for aprendizations
 - eventos unfocus?
 - servidor nao esta a lidar com eventos scroll/resize. ignorar?
 - Test.reset_answers() unused.
@@ -62,6 +61,8 @@ ou usar push (websockets?)
 # FIXED
+- questions.py textarea has a abspath which does not make sense! why is it there? not working for perguntations, but seems to work for aprendizations
+- textarea foi modificado em aprendizations para receber cmd line args. corrigir aqui tb.
 - usar npm para instalar javascript
 - se aluno entrar com l12345 rebenta. numa funcao get_... ver imagem no ipad
 - no test3 está contar 1.0 valores numa pergunta do tipo info? acontece para type: info, e não para type: information
@@ -149,7 +149,7 @@ Instead, tcp traffic can be forwarded from port 443 to 8443 where the server is
 listening.
 The details depend on the operating system/firewall.
-### debian:
+### debian
 FIXME: Untested
@@ -226,34 +226,34 @@ directory.
 ## Troubleshooting
-* The server tries to run `python3` so this command must be accessible from
+- The server tries to run `python3` so this command must be accessible from
 user accounts. Currently, the minimum supported python version is 3.6.
-* If you are getting any `UnicodeEncodeError` type of errors that's because the
+- If you are getting any `UnicodeEncodeError` type of errors that's because the
 terminal is not supporting UTF-8.
 This error may occur when a unicode character is printed to the screen by the
 server or, when running question generator or correction scripts, a message is
 piped between the server and the scripts that includes unicode characters.
 Try running `locale` on the terminal and see if there are any error messages.
 Solutions:
-    - debian: `sudo dpkg-reconfigure locales` and select your UTF-8 locales.
-    - FreeBSD: edit `~/.login_conf` to use UTF-8, for example:
+  - debian: `sudo dpkg-reconfigure locales` and select your UTF-8 locales.
+  - FreeBSD: edit `~/.login_conf` to use UTF-8, for example:
-        ```
-        me:\
-                :charset=UTF-8:\
-                :lang=en_US.UTF-8:
-        ```
+    ```text
+    me:\
+            :charset=UTF-8:\
+            :lang=en_US.UTF-8:
+    ```
 ---
-## Contribute ###
+## Contribute
-* Writing questions in yaml format
-* Testing and reporting bugs
-* Code review
-* New features and ideas
+- Writing questions in yaml format
+- Testing and reporting bugs
+- Code review
+- New features and ideas
-### Contacts ###
+### Contacts
-* Miguel Barão mjsb@uevora.pt
+- Miguel Barão mjsb@uevora.pt
@@ -32,10 +32,12 @@ async def check_password(try_pw, password):
     hashed = await loop.run_in_executor(None, bcrypt.hashpw, try_pw, password)
     return password == hashed
+
 async def hash_password(pw):
     pw = pw.encode('utf-8')
     loop = asyncio.get_running_loop()
-    return await loop.run_in_executor(None, bcrypt.hashpw, pw, bcrypt.gensalt())
+    r = await loop.run_in_executor(None, bcrypt.hashpw, pw, bcrypt.gensalt())
+    return r
 # ============================================================================
@@ -109,7 +111,9 @@ class App(object):
         # get name+password from db
         with self.db_session() as s:
-            name, password = s.query(Student.name, Student.password).filter_by(id=uid).one()
+            name, password = s.query(Student.name, Student.password)\
+                              .filter_by(id=uid)\
+                              .one()
         # first login updates the password
         if password == '':              # update password on first login
@@ -140,7 +144,8 @@ class App(object):
         if uid in self.online:
             logger.info(f'Student {uid}: started generating new test.')
             student_id = self.online[uid]['student']  # {number, name}
-            self.online[uid]['test'] = await self.testfactory.generate(student_id)
+            test = await self.testfactory.generate(student_id)
+            self.online[uid]['test'] = test
             logger.debug(f'Student {uid}: finished generating test.')
             return self.online[uid]['test']
         else:
@@ -156,7 +161,8 @@ class App(object):
         grade = await t.correct()
         # save test in JSON format
-        fname = ' -- '.join((t['student']['number'], t['ref'], str(t['finish_time']))) + '.json'
+        fields = (t['student']['number'], t['ref'], str(t['finish_time']))
+        fname = ' -- '.join(fields) + '.json'
         fpath = path.join(t['answers_dir'], fname)
         t.save_json(fpath)
@@ -189,9 +195,8 @@ class App(object):
         t.giveup()
         # save JSON with the test
-        # fname = ' -- '.join((t['student']['number'], t['ref'], str(t['finish_time']))) + '.json'
-        # fpath = path.abspath(path.join(t['answers_dir'], fname))
-        fname = ' -- '.join((t['student']['number'], t['ref'], str(t['finish_time']))) + '.json'
+        fields = (t['student']['number'], t['ref'], str(t['finish_time']))
+        fname = ' -- '.join(fields) + '.json'
         fpath = path.join(t['answers_dir'], fname)
         t.save_json(fpath)
@@ -225,22 +230,28 @@ class App(object):
     def get_student_grades_from_all_tests(self, uid):
         with self.db_session() as s:
-            return s.query(Test.title, Test.grade, Test.finishtime).filter_by(student_id=uid).order_by(Test.finishtime)
+            return s.query(Test.title, Test.grade, Test.finishtime)\
+                    .filter_by(student_id=uid)\
+                    .order_by(Test.finishtime)
     def get_json_filename_of_test(self, test_id):
         with self.db_session() as s:
-            return s.query(Test.filename).filter_by(id=test_id).scalar()
-
-    # def get_online_students(self):            # [('uid', 'name', 'starttime')]
-    #     return [(k, v['student']['name'], str(v.get('test', {}).get('start_time', '---'))) for k, v in self.online.items() if k != '0']
+            return s.query(Test.filename)\
+                    .filter_by(id=test_id)\
+                    .scalar()
     def get_all_students(self):
         with self.db_session() as s:
-            return s.query(Student.id, Student.name, Student.password).filter(Student.id != '0').order_by(Student.id)
+            return s.query(Student.id, Student.name, Student.password)\
+                    .filter(Student.id != '0')\
+                    .order_by(Student.id)
     def get_student_grades_from_test(self, uid, testid):
         with self.db_session() as s:
-            return s.query(Test.grade, Test.finishtime, Test.id).filter_by(student_id=uid).filter_by(ref=testid).all()
+            return s.query(Test.grade, Test.finishtime, Test.id)\
+                    .filter_by(student_id=uid)\
+                    .filter_by(ref=testid)\
+                    .all()
     def get_students_state(self):
         return [{
@@ -248,7 +259,8 @@ class App(object):
                 'name': name,
                 'allowed': uid in self.allowed,
                 'online': uid in self.online,
-                'start_time': self.online.get(uid, {}).get('test', {}).get('start_time', ''),
+                'start_time': self.online.get(uid, {}).get('test', {})
+                                         .get('start_time', ''),
                 'password_defined': pw != '',
                 'grades': self.get_student_grades_from_test(uid, self.testfactory['ref']),
                 # 'focus': self.online.get(uid, {}).get('student', {}).get('focus', True), # FIXME
@@ -32,9 +32,10 @@ import logging
 # this project
 from perguntations.tools import load_yaml, run_script
-from perguntations.questions import (QuestionRadio, QuestionCheckbox, QuestionText,
-                        QuestionTextRegex, QuestionNumericInterval,
-                        QuestionTextArea, QuestionInformation)
+from perguntations.questions import (QuestionRadio, QuestionCheckbox,
+                                     QuestionText, QuestionTextRegex,
+                                     QuestionNumericInterval, QuestionTextArea,
+                                     QuestionInformation)
 # setup logger for this module
 logger = logging.getLogger(__name__)
@@ -73,16 +74,18 @@ class QuestionFactory(dict):
     # After this, each question will have at least 'ref' and 'type' keys.
     # ------------------------------------------------------------------------
     def add_question(self, question):
+        q = question
+
         # if missing defaults to ref='/path/file.yaml:3'
-        question.setdefault('ref', f'{question["filename"]}:{question["index"]}')
+        q.setdefault('ref', f'{q["filename"]}:{q["index"]}')
-        if question['ref'] in self:
-            logger.error(f'Duplicate reference "{question["ref"]}".')
+        if q['ref'] in self:
+            logger.error(f'Duplicate reference "{q["ref"]}".')
-        question.setdefault('type', 'information')
+        q.setdefault('type', 'information')
-        self[question['ref']] = question
-        logger.debug(f'Added question "{question["ref"]}" to the pool.')
+        self[q['ref']] = q
+        logger.debug(f'Added question "{q["ref"]}" to the pool.')
     # ------------------------------------------------------------------------
     # load single YAML questions file
@@ -140,10 +143,11 @@ class QuestionFactory(dict):
         # which will print a valid question in yaml format to stdout. This
         # output is then converted to a dictionary and `q` becomes that dict.
         if q['type'] == 'generator':
-            logger.debug(f'Running script to generate question "{ref}".')
-            q.setdefault('arg', '')  # optional arguments will be sent to stdin
-            script = path.normpath(path.join(q['path'], q['script']))
-            out = run_script(script=script, stdin=q['arg'])
+            logger.debug(f'Generating "{ref}" from {q["script"]}')
+            q.setdefault('args', [])  # optional arguments
+            q.setdefault('stdin', '')
+            script = path.join(q['path'], q['script'])
+            out = run_script(script=script, args=q['args'], stdin=q['stdin'])
             try:
                 q.update(out)
             except Exception:
@@ -152,6 +156,7 @@ class QuestionFactory(dict):
                     'title': 'Erro interno',
                     'text': 'Ocorreu um erro a gerar esta pergunta.'
                     })
+
         # The generator was replaced by a question but not yet instantiated
         # Finally we create an instance of Question()
@@ -181,7 +181,7 @@ def main():
     for s in args.update:
         print(f'Updating password of: {s}')
         u = session.query(Student).get(s)
-        pw =(args.pw or s).encode('utf-8')
+        pw = (args.pw or s).encode('utf-8')
         u.password = bcrypt.hashpw(pw, bcrypt.gensalt())
         session.commit()
@@ -57,7 +57,7 @@ class Question(dict):
     def set_defaults(self, d):
         'Add k:v pairs from default dict d for nonexistent keys'
-        for k,v in d.items():
+        for k, v in d.items():
             self.setdefault(k, v)
@@ -74,7 +74,7 @@ class QuestionRadio(Question):
         choose (int)  # only used if shuffle=True
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
@@ -91,7 +91,12 @@ class QuestionRadio(Question):
         # always convert to list, e.g.  correct: 2 --> correct: [0,0,1,0,0]
         # correctness levels from 0.0 to 1.0 (no discount here!)
         if isinstance(self['correct'], int):
-            self['correct'] = [1.0 if x==self['correct'] else 0.0 for x in range(n)]
+            self['correct'] = [1.0 if x == self['correct'] else 0.0
+                               for x in range(n)]
+
+        if len(self['correct']) != n:
+            logger.error(f'Options and correct mismatch in '
+                         f'"{self["ref"]}", file "{self["filename"]}".')
         if self['shuffle']:
             # separate right from wrong options
@@ -102,8 +107,8 @@ class QuestionRadio(Question):
             # choose 1 correct option
             r = random.choice(right)
-            options = [ self['options'][r] ]
-            correct = [ 1.0 ]
+            options = [self['options'][r]]
+            correct = [1.0]
             # choose remaining wrong options
             random.shuffle(wrong)
@@ -113,10 +118,10 @@ class QuestionRadio(Question):
             # final shuffle of the options
             perm = random.sample(range(self['choose']), self['choose'])
-            self['options'] = [ str(options[i]) for i in perm ]
-            self['correct'] = [ float(correct[i]) for i in perm ]
+            self['options'] = [str(options[i]) for i in perm]
+            self['correct'] = [float(correct[i]) for i in perm]
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
@@ -145,7 +150,7 @@ class QuestionCheckbox(Question):
         answer (None or an actual answer)
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
@@ -154,14 +159,15 @@ class QuestionCheckbox(Question):
         # set defaults if missing
         self.set_defaults({
             'text': '',
-            'correct': [1.0] * n,     # Using 0.0 breaks the (right, wrong) options
+            'correct': [1.0] * n,   # Using 0.0 breaks the (right, wrong) opts
             'shuffle': True,
             'discount': True,
-            'choose': n,              # number of options
+            'choose': n,            # number of options
             })
         if len(self['correct']) != n:
-            logger.error(f'Options and correct size mismatch in "{self["ref"]}", file "{self["filename"]}".')
+            logger.error(f'Options and correct size mismatch in '
+                         f'"{self["ref"]}", file "{self["filename"]}".')
         # if an option is a list of (right, wrong), pick one
         # FIXME it's possible that all options are chosen wrong
@@ -169,9 +175,9 @@ class QuestionCheckbox(Question):
         correct = []
         for o, c in zip(self['options'], self['correct']):
             if isinstance(o, list):
-                r = random.randint(0,1)
+                r = random.randint(0, 1)
                 o = o[r]
-                c = c if r==0 else -c
+                c = c if r == 0 else -c
             options.append(str(o))
             correct.append(float(c))
@@ -182,7 +188,7 @@ class QuestionCheckbox(Question):
             self['options'] = [options[i] for i in perm]
             self['correct'] = [correct[i] for i in perm]
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
@@ -207,7 +213,7 @@ class QuestionCheckbox(Question):
         return self['grade']
-# ===========================================================================
+# ============================================================================
 class QuestionText(Question):
     '''An instance of QuestionText will always have the keys:
         type (str)
@@ -216,7 +222,7 @@ class QuestionText(Question):
         answer (None or an actual answer)
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
@@ -232,7 +238,7 @@ class QuestionText(Question):
         # make sure all elements of the list are strings
         self['correct'] = [str(a) for a in self['correct']]
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
@@ -252,7 +258,7 @@ class QuestionTextRegex(Question):
         answer (None or an actual answer)
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
@@ -261,15 +267,17 @@ class QuestionTextRegex(Question):
             'correct': '$.^',   # will always return false
             })
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
         if self['answer'] is not None:
             try:
-                self['grade'] = 1.0 if re.match(self['correct'], self['answer']) else 0.0
+                self['grade'] = 1.0 if re.match(self['correct'],
+                                                self['answer']) else 0.0
             except TypeError:
-                logger.error('While matching regex {self["correct"]} with answer {self["answer"]}.')
+                logger.error('While matching regex {self["correct"]} with '
+                             'answer {self["answer"]}.')
         return self['grade']
@@ -284,7 +292,7 @@ class QuestionNumericInterval(Question):
     An answer is correct if it's in the closed interval.
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
@@ -293,7 +301,7 @@ class QuestionNumericInterval(Question):
             'correct': [1.0, -1.0],   # will always return false
             })
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
@@ -321,7 +329,7 @@ class QuestionTextArea(Question):
         lines (int)
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
@@ -332,33 +340,34 @@ class QuestionTextArea(Question):
             'correct': ''  # trying to execute this will fail => grade 0.0
             })
-        # self['correct'] = path.join(self['path'], self['correct'])
-        self['correct'] = path.abspath(path.normpath(path.join(self['path'], self['correct'])))
+        self['correct'] = path.join(self['path'], self['correct'])
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
         if self['answer'] is not None:
-            # correct answer
             out = run_script(  # and parse yaml ouput
                 script=self['correct'],
+                args=self['args'],
                 stdin=self['answer'],
                 timeout=self['timeout']
                 )
-            if type(out) in (int, float):
-                self['grade'] = float(out)
-
-            elif isinstance(out, dict):
+            if isinstance(out, dict):
                 self['comments'] = out.get('comments', '')
                 try:
                     self['grade'] = float(out['grade'])
                 except ValueError:
-                    logger.error(f'Correction script of "{self["ref"]}" returned nonfloat.')
+                    logger.error(f'Grade value error in "{self["correct"]}".')
                 except KeyError:
-                    logger.error('Correction script of "{self["ref"]}" returned no "grade".')
+                    logger.error(f'No grade in "{self["correct"]}".')
+            else:
+                try:
+                    self['grade'] = float(out)
+                except (TypeError, ValueError):
+                    logger.error(f'Grade value error in "{self["correct"]}".')
         return self['grade']
@@ -370,14 +379,14 @@ class QuestionInformation(Question):
         text (str)
         points (0.0)
     '''
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     def __init__(self, q):
         super().__init__(q)
         self.set_defaults({
             'text': '',
             })
-    #------------------------------------------------------------------------
+    # ------------------------------------------------------------------------
     # can return negative values for wrong answers
     def correct(self):
         super().correct()
@@ -280,7 +280,8 @@ class ReviewHandler(BaseHandler):
         else:
             with f:
                 t = json.load(f)
-            self.render('review.html', t=t, md=md_to_html, templ=self._templates)
+            self.render('review.html', t=t, md=md_to_html,
+                        templ=self._templates)
 # --- ADMIN ------------------------------------------------------------------
@@ -406,7 +407,7 @@ def get_logger_config(debug=False):
             'level': level,
             'propagate': False,
             } for module in ['app', 'models', 'factory', 'questions',
-                             'test', 'tools', 'serve']})
+                             'test', 'tools']})
     return load_yaml(config_file, default=default_config)
@@ -465,8 +466,8 @@ def main():
     except ValueError:
         logging.critical('Certificates cert.pem, privkey.pem not found')
         sys.exit(-1)
-    else:
-        httpserver.listen(8443)
+
+    httpserver.listen(8443)
     # --- run webserver
     logging.info('Webserver running...  (Ctrl-C to stop)')
@@ -145,7 +145,7 @@ def md_to_html(qref=&#39;.&#39;):
 def load_yaml(filename, default=None):
     try:
         with open(path.expanduser(filename), 'r', encoding='utf-8') as f:
-            default = yaml.load(f)
+            default = yaml.safe_load(f)
     except FileNotFoundError:
         logger.error(f'File not found: "{filename}".')
     except PermissionError:
@@ -164,10 +164,11 @@ def load_yaml(filename, default=None):
 # The script is run in another process but this function blocks waiting
 # for its termination.
 # ---------------------------------------------------------------------------
-def run_script(script, stdin='', timeout=5):
+def run_script(script, args=[], stdin='', timeout=5):
     script = path.expanduser(script)
     try:
-        p = subprocess.run([script],
+        cmd = [script] + [str(a) for a in args]
+        p = subprocess.run(cmd,
                            input=stdin,
                            stdout=subprocess.PIPE,
                            stderr=subprocess.STDOUT,
@@ -187,7 +188,7 @@ def run_script(script, stdin=&#39;&#39;, timeout=5):
             logger.error(f'Script "{script}" returned code {p.returncode}')
         else:
             try:
-                output = yaml.load(p.stdout)
+                output = yaml.safe_load(p.stdout)
             except Exception:
                 logger.error(f'Error parsing yaml output of "{script}"')
             else:
	@@ -181,7 +181,7 @@ def main():		@@ -181,7 +181,7 @@ def main():
181	for s in args.update:	181	for s in args.update:
182	print(f'Updating password of: {s}')	182	print(f'Updating password of: {s}')
183	u = session.query(Student).get(s)	183	u = session.query(Student).get(s)
184	- pw =(args.pw or s).encode('utf-8')	184	+ pw = (args.pw or s).encode('utf-8')
185	u.password = bcrypt.hashpw(pw, bcrypt.gensalt())	185	u.password = bcrypt.hashpw(pw, bcrypt.gensalt())
186	session.commit()	186	session.commit()
187		187