Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

Warn in case of information loss from Unmarshal #356

Open
glooms opened this issue Jul 11, 2019 · 3 comments
Open

Warn in case of information loss from Unmarshal #356

glooms opened this issue Jul 11, 2019 · 3 comments
Labels
bug Something isn't working enhancement New feature or request v2

Comments

@glooms
Copy link
Contributor

glooms commented Jul 11, 2019

Go's default json.Unmarshal disregards any information that could not be properly mapped to the provided container (struct/slice/map). If we are working with an engine version not compatible with our enigma version it is plausible we would get information loss which is dangerous in a version control or unbuild/build scenario.

We should at the very least warn the user if information loss has occurred.

@glooms glooms added bug Something isn't working enhancement New feature or request labels Jul 11, 2019
@glooms
Copy link
Contributor Author

glooms commented Jul 19, 2019

There seems to be an easy way to do this, but it has a couple of key limitations that might make it poorly suited for our case.

Using the builtin Decoder from encoding/json you can set DisallowUnknownFields() which will cause the decoder to return an error if there is data that is not mapped to the field of a struct. The drawback however is that it only returns the first unknown field, meaning we have no way of knowing how many such fields there are.

@glooms
Copy link
Contributor Author

glooms commented Jul 22, 2019

It seems like this doesn't happen as this is the same as having unrecognized fields (if I'm not mistaken).
I'll leave this open until clarification from @wennmo and @gabbaxx

@glooms
Copy link
Contributor Author

glooms commented Sep 2, 2019

This will only be an issue while using the --minimum flag and under the specific circumstance that corectl uses a version of enigma that was generated for a QIX-api version older that of the engine which the user is using.

We could raise a warning, if this is the case.

@wennmo wennmo added the v2 label Sep 9, 2019
@glooms glooms removed their assignment Mar 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement New feature or request v2
Projects
None yet
Development

No branches or pull requests

2 participants