Skip to content

Conversation

aothms
Copy link
Collaborator

@aothms aothms commented Sep 12, 2025

We don't check that the subprocess actually succeeds (returns 0 exit code)

Propagating the typed exception (I think that was my hack) doesn't work because of the CalledProcessError constructor.

Traceback (most recent call last):
  File "/mnt/c/Users/tkrij/Documents/AECgeeks/projects/buildingsmart/validate/backend/apps/ifc_validation/tasks/task_runner.py", line 130, in validation_subtask_runner
    context = config.check_program(TaskContext(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/c/Users/tkrij/Documents/AECgeeks/projects/buildingsmart/validate/backend/apps/ifc_validation/tasks/check_programs.py", line 48, in check_schema       
    proc = run_subprocess(
           ^^^^^^^^^^^^^^^
  File "/mnt/c/Users/tkrij/Documents/AECgeeks/projects/buildingsmart/validate/backend/apps/ifc_validation/tasks/check_programs.py", line 225, in run_subprocess    
    raise type(err)(f"Unknown error during validation task {task.id}: {task.type}") from err
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: CalledProcessError.__init__() missing 1 required positional argument: 'cmd'

@aothms aothms requested a review from Ghesselink September 12, 2025 09:51
@aothms
Copy link
Collaborator Author

aothms commented Sep 12, 2025

Maybe not to be merged as is, maybe some subprocesses, such as ifcopenshell.validate, return a non-zero statuscode if there are validation errors (probably 1) then. But any unknown exceptions should be handled. Maybe we should whitelist either 0 or 1, or make this expected statuscodes dependent on the program. But unhandled things somehow have to bubble up to us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant