Requests from TK

[aothms]

The following request/questions came from a quick interaction with the datamodel.

1. IfcValidationOutcome.instance points to model instead of instance

ifc-validation-data-model/models.py

Lines 705 to 706 in d09fb3a

	instance = models.ForeignKey(
	to=IfcModel,

2. IfcValidationOutcome.instance should be optional. In not all cases we will have an instance to refer to

3. IfcValidationOutcome.feature and code are redundant. Delete code.

ifc-validation-data-model/models.py

Lines 725 to 730 in d09fb3a

	feature = models.CharField(
	max_length=1024,
	null=False,
	blank=False,
	help_text="Name of the Gherkin Feature."
	)

ifc-validation-data-model/models.py

Lines 739 to 745 in d09fb3a

	code = models.CharField(
	max_length=6,
	null=False,
	blank=False,
	db_index=True,
	help_text="Name of the Gherkin Feature."
	)

4. IfcValidationOutcome.feature and feature_version Should be optional as not everything is Gherkin-related.

5. My preference would be to remove the Ifc- prefix from all classes

6. IfcModel has no attribute type, suggest file_name. I didn't review all repr functions, just came across this in my repl session.

ifc-validation-data-model/models.py

Line 381 in d09fb3a

return f'#{self.id} - {self.created} - {self.type}'

7. IfcModel.Status: Needed? Or retrieve as related IfcValidationTask.Status? We have it currently in the datamodel, but in my mind it is a bit redundant.

ifc-validation-data-model/models.py

Lines 185 to 191 in d09fb3a

	class Status(models.TextChoices):
	"""
	The overall status of a Model.
	"""
	VALID = 'v', 'Valid'
	INVALID = 'i', 'Invalid'
	NOT_VALIDATED = 'n', 'Not Validated'

ifc-validation-data-model/models.py

Lines 280 to 288 in d09fb3a

	status_bsdd = models.CharField(
	max_length=1,
	choices=Status.choices,
	default=Status.NOT_VALIDATED,
	db_index=True,
	null=False,
	blank=False,
	help_text="Status of the bSDD Validation."
	)

etc.

vs.

ifc-validation-data-model/models.py

Lines 554 to 563 in d09fb3a

	class Status(models.TextChoices):
	"""
	The overall status of a Validation Task.
	"""
	PENDING = 'PENDING', 'Pending'
	SKIPPED = 'SKIPPED', 'Skipped'
	NOT_APPLICABLE = 'N/A', 'Not Applicable'
	INITIATED = 'INITIATED', 'Initiated'
	FAILED = 'FAILED', 'Failed'
	COMPLETED = 'COMPLETED', 'Completed'

8. IfcValidationTask vs IfcValidationRequest. Why both?

ifc-validation-data-model/models.py

Line 434 in d09fb3a

class IfcValidationRequest(AuditedBaseModel):

ifc-validation-data-model/models.py

Line 535 in d09fb3a

class IfcValidationTask(AuditedBaseModel):

9. IfcValidationTask/IfcValidationRequest. No foreign key to IfcModel

[rw-bsi]

Fixes

1. IfcValidationOutcome.instance points to model instead of instance

FIXED

2. IfcValidationOutcome.instance should be optional. In not all cases we will have an instance to refer to

UPDATED

3. IfcValidationOutcome.feature and code are redundant. Delete code.

UPDATED

~~ 4. IfcValidationOutcome.feature and feature_version Should be optional as not everything is Gherkin-related.~~

UPDATED

5. My preference would be to remove the Ifc- prefix from all classes

TBC - I see pro/con, mainly driven by what I saw in ifc standard when I was getting familiar with the domain, but can see that it will be confusing and will potentially lead to name clashes. Will replace it, yet in the table names I still prefer a ifc_ prefix

ifc-validation-data-model/models.py

Lines 374 to 377 in 29cd217

	class Meta:
	db_table = "ifc_model"
	verbose_name = "Model"
	verbose_name_plural = "Models"

6. IfcModel has no attribute type, suggest file_name. I didn't review all repr functions, just came across this in my repl session.

UPDATED (for IfcModel; will update repr where you and I see fit; mainly used in Django Admin for now)

7. IfcModel.Status: Needed? Or retrieve as related IfcValidationTask.Status? We have it currently in the datamodel, but in my mind it is a bit redundant.

Added it initially for performance/backwards compatibility reasons - can be removed

8. IfcValidationTask vs IfcValidationRequest. Why both?

one request leads to multiple sub- tasks

9. IfcValidationTask/IfcValidationRequest. No foreign key to IfcModel

UPDATED: added request.model; Tasks can navigate via task.request.model

[aothms]

TBC - I see pro/con, mainly driven by what I saw in ifc standard when I was getting familiar with the domain, but can see that it will be confusing and will potentially lead to name clashes. Will replace it, yet in the table names I still prefer a ifc_ prefix

That's ok. I think it's especially the combination of IfcPascalCase style with IfcPrefix that may lead to confusion as to whether we're talking about concepts from the IFC schema or from our own application. E.g IfcApplication is from the schema, we've called it an IfcAuthoringTool, but it could have easily collided.

one request leads to multiple sub- tasks

Ok that's what I thought and I think that's good, so we can rerun requests on models. But I think the current fields on Request make it a bit confusing as it duplicates fields from both model and task.

Model
  |
  |
  | 1..*
  v
Request
  | 
  | 
  | 7
  v 
Task

May I suggest:

• Remove:
  • file_name, file (available on model)
  • status, status_reason, progress (available on task)
• Provide a getter on Model to return Request with the highest creation_date.

Later we can:

• Add a started_reason enum (UPLOAD, AUTOMATIC_RETRY, MANUAL_RERUN, ...)
• Compute aggregated progress from tasks. Based on automatically accumulated metrics on how long the various tasks take.