mozilla / active-data-recipes

A repository of various activedata queries and recipes
Mozilla Public License 2.0
9 stars 24 forks source link

Separating query contexts and run contexts - Update 16 recipes #86

Closed TrangNguyenBC closed 5 years ago

TrangNguyenBC commented 5 years ago

This PR continues from #85 to solve issue #40 with these changes: These recipes were updated following new structure: 'activedata_usage', 'backout_rate', 'code_coverage', 'code_coverage_by_suite', 'config_durations', 'files_with_coverage', 'inspect', 'seta_accuracy', 'skipped_tests', 'task_durations', 'tests_config_times', 'test_durations', 'tests_in_duration', 'try_efficiency', 'try_usage', 'try_users'

TrangNguyenBC commented 5 years ago

Hi @ahal, in most cases, I removed the part of setting up query contexts and parsing query contexts in run() function of recipe file to adapt the new structure. These are some cases I made some more changes in related files (listed by recipe name):

I also have problems changing these other recipes:

Please let me know how you think about it and what I should change. Thanks.

TrangNguyenBC commented 5 years ago

Hi @ahal, I removed all duplicated functions. I still have problem updating these 3 recipes: intermittent_test_data, intermittent_tests, raw_coverage. The problem of two "intermittent" recipes were described in my previous comment. With raw_coverage recipe, there is a problem with {$eval: path+'/[A-Za-z0-9]+/.*\"'}. The error message is yaml.parser.ParserError: while parsing a flow mappingin expected ',' or '}', but got '['. Please help me to have a look and give me some guides. Thank you!

ahal commented 5 years ago

With raw_coverage recipe, there is a problem with {$eval: path+'/[A-Za-z0-9]+/.*\"'}. The error message is yaml.parser.ParserError: while parsing a flow mappingin expected ',' or '}', but got '['. Please help me to have a look and give me some guides. Thank you!

It looks like raw_coverage.py is broken at the moment anyway. I talked to jmaher, and he wouldn't mind if we just removed it. Though I guess this error could still potentially happen in a future recipe if we don't fix it. Though it's likely the query just needs to use a different combination of quotes or escape characters.

But either way, I think it's ok if you just ignore this one for now (though please file an issue so we remember about it later).

ahal commented 5 years ago

I also have problems changing these other recipes:

* The value of some contexts depends on value of a context. For example, in the case of `intermittent_test_data` recipe, values of 4 contexts `test`, `platform_config`, `groupby`, `result` are set in `run()` function based on the input value (in some case, default value) of context `test`

* The value of a context are calculating from other contexts and transparent to user. For example, in the case of `intermittent_test_data` recipe, the `platform_config` context is listed in name of query context, but there is no definition in query file because it will be generated later in `run()` function by using value of other contexts (`platform` and `build_type`)  or giving a value; depends on value of context `test` as mentioned in previous item. If user uses `--help` with this recipe, s/he will see nothing about `platform_config`.
  If `platform_config` is always generated by concatenate `platform` and `build_type`, I guess we can replace `{$eval: platform_config}` by `{$eval: platform + build_type}`, but in this case it values depends on `test` value, so it is more complicated. Currently I have no solution for it.

Please let me know how you think about it and what I should change. Thanks.

Yes, there is also the case where the output of query A is the input for query B.

I think we're going to need to go back to passing the context into the run function, and then manually forwarding it to each execute_query call. This will allow recipes to modify the context in-between calls if they need to. We can still require the query to provide a default value for every piece of context though, so that it can be executed stand-alone. A tip which might come in handy is that we can hide an argument from the command line by setting help=argparse.SUPPRESS. This might be useful for, e.g hiding values like platform_config from the --help text.

Does this make sense?