PR#121: Tuples only, please. - taskotron/resultsdb

taskotron / resultsdb

#121 Tuples only, please.

Closed 5 years ago by jskladan. Opened 5 years ago by ralph.

taskotron/ ralph/resultsdb tuple-not-list into develop

Tuples only, please.

Ralph Bean • 5 years ago

493ed89

resultsdb/models/results.py

file modified

+2 -1

		`@@ -27,7 +27,8 @@`
		`__all__ = ['Testcase', 'Group', 'Result', 'ResultData', 'GroupsToResults', 'RESULT_OUTCOME']`

		`PRESET_OUTCOMES = ('PASSED', 'INFO', 'FAILED', 'NEEDS_INSPECTION')`
		`- RESULT_OUTCOME = PRESET_OUTCOMES + app.config.get('ADDITIONAL_RESULT_OUTCOMES', [])`
		`+ ADDITIONAL_RESULT_OUTCOMES = tuple(app.config.get('ADDITIONAL_RESULT_OUTCOMES', []))`
		`+ RESULT_OUTCOME = PRESET_OUTCOMES + ADDITIONAL_RESULT_OUTCOMES`
		`JOB_STATUS = []`

ralph commented 5 years ago

This gets us out of a TypeError we're hitting:

TypeError: can only concatenate tuple (not "list") to tuple

lholecek commented 5 years ago

Would be nice not to restrict the configuration value to tuples only. E.g.:

ADDITIONAL_RESULT_OUTCOMES = app.config.get('ADDITIONAL_RESULT_OUTCOMES', [])
RESULT_OUTCOME = tuple(itertools.chain(PRESET_OUTCOMES, ADDITIONAL_RESULT_OUTCOMES))

jskladan commented 5 years ago

@ralph this IMO is not really a proper solution. AFAIU this will break again, when you set the ADDITIONAL_RESULT_OUTCOMES as list, instead of tuple in the config file, so it lacks some robustness to me (as noted by @lholecek)

@lholecek I might be missing something, but why use tuple(itertools.chain(...)) instead of RESULT_OUTCOME = PRESET_OUTCOMES + tuple(ADDITIONAL_RESULT_OUTCOMES)? Not that I have a huge issue with that, just don't see the added value.

lholecek commented 5 years ago

@jskladan The original problem was:

>>> list() + tuple()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can only concatenate list (not "tuple") to list

lholecek commented 5 years ago

@jskladan Oh, sorry, disregard my previous comment. I think you understood it. Your solution is probably nicer (RESULT_OUTCOME = PRESET_OUTCOMES + tuple(ADDITIONAL_RESULT_OUTCOMES)).

ralph commented 5 years ago

Will adjust. Thanks guys!

rebased onto 493ed89

5 years ago

ralph commented 5 years ago

OK. Updated.

jskladan commented 5 years ago

Awesome, thanks! /merging

ralph commented 5 years ago

\ó/

jskladan commented 5 years ago

Merged by https://pagure.io/taskotron/resultsdb/c/70c1ec66bb0b4b827f8f49f19c6fdaec2c61b892?branch=develop