spec-parser run reports errors but exits with `0`

[maxhbr]

The current spec parser reports an error but does still exit with EXIT_SUCCESS. See for example https://github.com/spdx/spdx-3-model/actions/runs/7118823989/job/19382571564#step:5:25 with the log:

 > Run cd spec-parser
WARNING : External property restrictions aren't yet handled properly, they are added to the description of the class.
WARNING : skipping ../model/Core/Properties/_property.md
WARNING : skipping ../model/Core/Vocabularies/_vocab.md
WARNING : External property restrictions aren't yet handled properly, they are added to the description of the class.
WARNING : External property restrictions aren't yet handled properly, they are added to the description of the class.
WARNING : External property restrictions aren't yet handled properly, they are added to the description of the class.
WARNING : External property restrictions aren't yet handled properly, they are added to the description of the class.
WARNING : Overwriting out_dir `/tmp/spec-parser.out`
ERROR   : Invalid Nature attribute in metadata `None`

[goneall]

In looking at the code, the spec-parser continues to run after the "Error".

Perhaps these are just warnings?

If they are indeed errors - should we stop processing and flag the error?

For the CI, I think it would be useful to report these as errors.

It looks like the methods that log the error all return a None value.

If we want to continue execution rather than raise an exception, we can change the return value to an integer which would be a return code - 0 being success. Then can then be used to set the system level return code.

@zvr @maxhbr - if you agree with the above and would like some help, I can write a PR.

[bact]

Does this already resolved in the current version of spec-parser?

[stevenc-stb]

I can confirms that it still occur
note from tech meeting from @bact "
We need non-zero exit from the spec-parser, so the GitHub workflow can know that there’s something wrong.

From spec-parser code,
this line detect the error and log it:

spec-parser/spec_parser/model.py

Lines 100 to 101 in 0141858

	if proptype != ptype and (not p.startswith("/") or proptype.rpartition("/")[-1] != ptype.rpartition("/")[-1]):
	logger.error(f"In class {c.fqname}, property {p} has type {ptype} but the range of {pname} is {proptype}")

And maybe this line is trying to check if there's any error in the log and exit(1) if there is:

spec-parser/runparams.py

Lines 23 to 24 in 0141858

	if logging.ERROR in self.log._cache:
	sys.exit(1)

But looks like it does not work (or I misunderstood the code flow).

We need non-zero exit from the spec-parser, so the GitHub workflow can know that there’s something wrong."

[bact]

Thank you @stevenc-stb. Look at the code again, I think we may need to check the log after these following lines (since the RunParams is more of the setup but Model object is where the generation is):

spec-parser/main.py

Lines 11 to 13 in 0141858

	m = Model(cfg.input_path)
	if not cfg.no_output:
	m.generate(cfg)

For the message from the PR validation workflow, please see https://github.com/spdx/spdx-3-model/actions/runs/15499592768/job/43644274069?pr=1029#check-step-5 (at step 5 "run spec-parser to validate")

[bact]

This PR #187 is addressing this issue.

The current code already logged errors.
The PR just checking in the end of the program if number of errors is more than zero. If it is, return 1.

[zvr]

I think, now that #188 has been merged, this has been taken care of.