DataModel Plugin #2494
DataModel Plugin #2494
Conversation
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
Signed-off-by: 0ssigeno <[email protected]>
|
The errors in the test are not related to the pr, you can review it |
![code-review-doctor[bot]](https://avatars.githubusercontent.com/in/83430?s=60&v=4){kind=small}
api_app/analyzers_manager/models.py
Outdated
| logger.info( | ||
| f"Skipping data model of {self.config.name} for job {self.config.pk} because no data model" | ||
| ) |
There was a problem hiding this comment.
| logger.info( | |
| f"Skipping data model of {self.config.name} for job {self.config.pk} because no data model" | |
| ) | |
| logger.info( | |
| f"Skipping data model of {self.config.name} for job {self.config_id} because no" | |
| " data model" | |
| ) |
api_app/analyzers_manager/models.py
Outdated
| def _validation_before_data_model(self) -> bool: | ||
| if not self.config.mapping_data_model: | ||
| logger.info( | ||
| f"Skipping data model of {self.config.name} for job {self.config.pk} because no data model" |
There was a problem hiding this comment.
self.config.pk performs a database read when pk is evaluated. You could take advantage of Django's caching of related fields by using self.config_id, which does not do a database read. More info.
| logger.info( | ||
| f"Skipping data model of {self.config.name} for job {self.config.pk} because status is " | ||
| f"{self.status}" | ||
| ) |
There was a problem hiding this comment.
| logger.info( | |
| f"Skipping data model of {self.config.name} for job {self.config.pk} because status is " | |
| f"{self.status}" | |
| ) | |
| logger.info( | |
| f"Skipping data model of {self.config.name} for job {self.config_id} because status" | |
| f" is {self.status}" | |
| ) |
| return False | ||
| if not self.status == self.STATUSES.SUCCESS.value: | ||
| logger.info( | ||
| f"Skipping data model of {self.config.name} for job {self.config.pk} because status is " |
There was a problem hiding this comment.
Again, consider self.config_id
|
|
||
| class Signature(models.Model): | ||
| provider = LowercaseCharField(max_length=100) | ||
| url = models.URLField(default=None, null=True, blank=True) |
There was a problem hiding this comment.
| url = models.URLField(default=None, null=True, blank=True) | |
| url = models.URLField(default="", blank=True) |
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Explained here.
|
|
||
|
|
||
| class DomainDataModel(BaseDataModel): | ||
| ietf_report = models.ForeignKey( |
There was a problem hiding this comment.
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. More details.
|
|
||
|
|
||
| class IPDataModel(BaseDataModel): | ||
| ietf_report = models.ForeignKey( |
There was a problem hiding this comment.
Same as above: with an explicit related_name would be better.
Signed-off-by: 0ssigeno <[email protected]>
|
|
||
|
|
||
| class DataModelEvaluations(Choices): | ||
| FALSE_POSITIVE = "false_positive" |
There was a problem hiding this comment.
| FALSE_POSITIVE = "false_positive" | |
| TRUSTED = "trusted" |
(Please add to the PR name the issue/s that this PR would close if merged by using a Github keyword. Example:
<feature name>. Closes #999. If your PR is made by a single commit, please add that clause in the commit too. This is all required to automate the closure of related issues.)Description
Please include a summary of the change and link to the related issue.
Type of change
Please delete options that are not relevant.
Checklist
developdumpplugincommand and added it in the project as a data migration. ("How to share a plugin with the community")test_files.zipand you added the default tests for that mimetype in test_classes.py.FREE_TO_USE_ANALYZERSplaybook by following this guide.urlthat contains this information. This is required for Health Checks._monkeypatch()was used in its class to apply the necessary decorators.MockUpResponseof the_monkeypatch()method. This serves us to provide a valid sample for testing.Black,Flake,Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.testsfolder). All the tests (new and old ones) gave 0 errors.DeepSource,Django Doctorsor other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.Important Rules