apply_changes: better exceptions

[miili]

Related to #570 #324.

I improved the DeltaException message to see what is going on and why the deltas fail erratically. I don't know QGIS Python well and wonder is there is an error() method analog to the layer.dataProdiver().error(). That could be helpful.

Took the liberty and rename delta_log to DELTA_LOG, as it is a global variable which is being reused. The global state here is a dangerous thing. Maybe pass along a lightweight context and fill it with information.

Other things I want to comment on:

• Don't use TypedDict. Go for dataclass where possible or even pydantic if you need serialisation/deserialistion.
• f-strings in log messages is discouraged, use %s syntax instead.
• Use Literal instead of Enum where feasible.

[suricactus]

Hey, @miili , thanks for the PR.

The cleanup you did is something I appreciate and it is mergeable.

Note the typing comments are relevant by today, but not when this script was started ;) . Let's leave them for a better day, actually I have some other ideas for this script in the future.

Took the liberty and rename delta_log to DELTA_LOG, as it is a global variable which is being reused. The global state here is a dangerous thing. Maybe pass along a lightweight context and fill it with information.

It was considered to pass the context around in the past, it was a bit cumbersome to deal with at the time, but we might reconsider it.

---

Now to the point of this PR. The change of the message you did is a bit misleading. The reason why we check for invalid function is because get_feature always returns QgsFeature. If a new instance of QgsFeature is created, by default it is invalid. That being said, returning an invalid QgsFeature might be because:

• getFeatures failed to find the targeted feature and we return the default QgsFeature (line 1031)
• the provider fails to read the feature from the storage and it is invalid

At first glance it makes sense we return None if no feature is found and distinguish the two scenarios above, but this might have other implications and needs a bit more thinking and digging in the history. But note, changing the get_feature part will take longer to get reviewed, tested, merged and deployed.

To summarize, please remove the changes related to the delta exception and we can merge this PR.

[miili]

I made some changes to get_feature, which now raises the exceptions FeatureNotFoundError and DoublicateFeatureError. Thus a validated QgsFeature is always returned.

Returning a None can be complicated as the caller has to to validation and a None can lead to bad and untraceable behavior. I strongly advocate against that pattern and rather use an explicit and meaningful try-except.

[suricactus]

As I said, changing the get_feature will take longer to get reviewed, tested, merged and deployed. Will get back to you when clean up the immediate backlog.