-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initialize ExportData with tmp config obj from AggregatedData. #626
Initialize ExportData with tmp config obj from AggregatedData. #626
Conversation
Ref options listed in the issue, I am not convinced that this is the best way of solving this. It feels a bit like duct tape. Open for opinions and suggestions. |
yes, it feels a bit hacky which is why I took it away recently 🙂 but I see the problem with logs being spammed. This AggregatedData needs a bit of attention later, but for now I guess taking this fake config back could be an option.. Or probably better, rewrite: fmu-dataio/src/fmu/dataio/dataio.py Line 475 in bec7647
to: That will silence the pydantic warnings, and there is no need of validating the config if it is not given anyway 👍 |
And then we move the warning regarding missing config and that the metadata will be empty to when the user try to create metadata instead. So that when using this AggregatedData, the users are not exposed to it. |
Yes, that looks potentially more elegant. But are there unwanted side-effects? Wouldn't this fail to warn if Edit: This will then fail to trigger:
|
Hm, maybe... And eventually we are moving towards requiring a valid config as input, so probably making the fake config here is the best solution for now 👍 and then we'll address it later when we give some attention to this class 🙂 |
Ah, did not see the last comment until now. Tried the (previously) suggested change. Can revert back if we liked the first one better. |
Which one do we want? First or second...? |
I say first! |
Backtracked, now using fake config again ✅ |
@tnatt happy? OK to merge...? |
Yes 👍 |
Solve #625 and silence warning about erroneous config. It is spamming the logs on our service, and the text is misleading (see issue).