Catch config upload validation errors

[craddm]

✅ Checklist

• You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
• You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
• Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).

🚦 Depends on

⤴️ Summary

Catches an exception when trying to upload an invalid config file and exits cleanly. Also improves the related error message to make it clearer what the validation error is.

Original error message:

Catches the error cleanly and yields a clearer error message

🌂 Related issues

Closes #2210

🔬 Tests

Tested locally

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
data_safe_haven/commands
config.py
Project Total

This report was generated by python-coverage-comment-action

[JimMadge]

Looks good. I think the output here is clearer than before.

@craddm we could also add a couple of tests here

1. To ensure a config problem give the human-friendly error
2. To check the format of the exception in yaml_serialisable_model.py

[JimMadge]

Looks like a test is trying to use az CLI. Maybe a missing mock fixture.

[craddm]

Looks like a test is trying to use az CLI. Maybe a missing mock fixture.

Yes - odd as it was working before we changed things today, and the only thing we changed which touches this test is how the yaml with the missing field is made.

[JimMadge]

Might be due to a change on develop that was merged in?

[craddm]

Might be due to a change on develop that was merged in?

Nope - just tested it out. It works fine if I revert the change in how sre_config_yaml_missing_field is made. Can't understand why!

[JimMadge]

Hmm 🤔 something about how the sre_config_yaml fixture is constructed?

[JimMadge]

Or, the change from missing value to missing field has changed what is happening underneath?

[JimMadge]

In any case, you should be able to follow the traceback (or code) to find out what is happening.

[craddm]

We just weren't doing it correctly, so the yaml wasn't actually invalid and the upload function was progressing to the point where it needed to be logged in. The validation errors happen before that point, so it isn't necessary to do extra mocks to cope with logins etc.

[JimMadge]

Oh, that is brilliant. We were too good at not writing bad YAML 😆.