SanityBot #435
SanityBot #435
Conversation
| @@ -0,0 +1,32 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
won't merge this---will wait for @paopow 's PR, rebase, and rebuild migrations.
orchestra/bots/sanitybot.py
Outdated
|
|
||
|
|
||
| def _workflow_versions_with_sanity_checks(): | ||
| # TODO(marcua): make more specific. |
There was a problem hiding this comment.
This doesn't incur serious performance issues in practice, so I'm tempted to leave as-is :)
Makefile
Outdated
| flake8 . && isort --check-only --recursive . | ||
| # TODO(marcua): reenable isort when it works the same in dev and | ||
| # on CircleCI | ||
| flake8 . # && isort --check-only --recursive . |
| @@ -0,0 +1,32 @@ | |||
| # -*- coding: utf-8 -*- | |||
| @@ -64,13 +65,16 @@ class WorkflowVersion(WorkflowVersionMixin, models.Model): | |||
| A longer description of the workflow version. | |||
| workflow (orchestra.models.Workflow): | |||
| The workflow that this is a version of. | |||
| sanity_checks (str): | |||
| A JSON blob used to define the sanity checks we run. | |||
There was a problem hiding this comment.
Where do I find an example of the format of this blob?
There was a problem hiding this comment.
reasonable question, updated docstring
orchestra/tests/helpers/workflow.py
Outdated
| }], | ||
| "repetition_seconds": 86400 | ||
| }, | ||
| } |
There was a problem hiding this comment.
Can we use this as an example of the sanity_checks blob? What would it look like with multiple checks? What happens to "deleted" checks?
orchestra/bots/sanitybot.py
Outdated
| .filter(project=project) | ||
| .values('check_slug') | ||
| .annotate(max_created_at=Max('created_at'))) | ||
| latest_check_creation = { |
There was a problem hiding this comment.
Reusing the variable name is OK?
orchestra/bots/sanitybot.py
Outdated
| check['check_slug']: check['max_created_at'] | ||
| for check in latest_check_creation} | ||
| for check in checks: | ||
| max_created_at = latest_check_creation.get(check.check_slug) |
There was a problem hiding this comment.
What is the different between check['check_slug'] and check.check_slug?
There was a problem hiding this comment.
check is of type SanityCheck (we both miss typed languages :)), which has a property called check_slug
latest_check_creation is a dictionary, which has keys defined by the set of distinct check_slug
Makefile
Outdated
| flake8 . && isort --check-only --recursive . | ||
| # TODO(marcua): reenable isort when it works the same in dev and | ||
| # on CircleCI | ||
| flake8 . # && isort --check-only --recursive . |
| """ | ||
| created_at = models.DateTimeField(default=timezone.now) | ||
| slug = models.CharField(max_length=200) | ||
| name = models.CharField(max_length=200) | ||
| description = models.TextField() | ||
| workflow = models.ForeignKey( | ||
| Workflow, related_name='versions', on_delete=models.CASCADE) | ||
| sanity_checks = JSONField(default={}) |
There was a problem hiding this comment.
Why storing a lot of information here instead of the SanityCheck itself?
orchestra/bots/sanitybot.py
Outdated
| .filter(project=project) | ||
| .values('check_slug') | ||
| .annotate(max_created_at=Max('created_at'))) | ||
| latest_check_creation = { |
orchestra/bots/sanitybot.py
Outdated
| check['check_slug']: check['max_created_at'] | ||
| for check in latest_check_creation} | ||
| for check in checks: | ||
| max_created_at = latest_check_creation.get(check.check_slug) |
There was a problem hiding this comment.
What is the difference between check['check_slug'] and check.check_slug?
| .get('path')) | ||
| check_configurations = sanity_checks.get('check_configurations') | ||
| if sanity_check_path and check_configurations: | ||
| sanity_check_function = locate(sanity_check_path) |
There was a problem hiding this comment.
it will throw an exception, which will stop the world. this has been how we've handled all *_function paths.
longer-term, we should consider 'statically analyzing' workflow configurations on load.
| check_configurations = sanity_checks.get('check_configurations') | ||
| if sanity_check_path and check_configurations: | ||
| sanity_check_function = locate(sanity_check_path) | ||
| sanity_checks = sanity_check_function(project) |
There was a problem hiding this comment.
What if the function return an invalid value?
There was a problem hiding this comment.
in general it gets "sanity checked" in _handle_sanity_checks(. Can you think of ways it'd break?
| sanity_check.save() | ||
|
|
||
|
|
||
| def create_and_handle_sanity_checks(): |
There was a problem hiding this comment.
In a deployed instance, when is this function called? How do we automatically check and handle the SanityCheck?
Addresses #434