Rewrite model check as validation check re #10079 #11646
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This model check should have been skippped by checking the `databases` kwarg provided when `check()` is called, but that reduces the circumstances on which this check is run to either: - manage.py migrate - manage.py check --databases So, given how infrequently this would be run, better to just implement this in our validation framework.
jacobtylerwalls
commented
Nov 21, 2024
| check=IntegrityCheck.PUBLICATION_MISSING_FOR_LANGUAGE, # 1014 | ||
| queryset=( | ||
| models.GraphModel.objects.filter( | ||
| isresource=True, |
There was a problem hiding this comment.
The prior version of this was also checking branches when it shouldn't.
This was referenced Nov 21, 2024
jacobtylerwalls
commented
Nov 21, 2024
| @@ -613,39 +613,6 @@ def save(self, *args, **kwargs): | |||
|
|
|||
| super(GraphModel, self).save(*args, **kwargs) | |||
|
|
|||
| @classmethod | |||
There was a problem hiding this comment.
Pretty sure this deletion is the main reason the coverage check is failing.
Some more info about the original motivation here: #11342 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Types of changes
Description of Change
When trying my hand at writing a django system check in #10079, I missed a pretty major caveat, which is that if you use the database, you need to do something like:
arches/arches/app/models/models.py
Lines 619 to 620 in 0f23a6c
... to avoid db hits during commands that don't generally expect a database to be configured. (My earlier workaround was was to run subsets of checks in different places, but that was kind of a code smell. This PR removes some of those workarounds.)
But to start observing
kwargs["databases"]means that these checks only run on:So, given how infrequently this would be run, better to just implement this in our validation framework and call it out in the release notes?
Testing instructions