Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (6a8fce3) 82.44% compared to head (1fab23d) 82.35%.

❗ Current head 1fab23d differs from pull request most recent head 6396689. Consider uploading reports for the commit 6396689 to get more accurate results

@@            Coverage Diff             @@
##             main     #382      +/-   ##
==========================================
- Coverage   82.44%   82.35%   -0.09%     
==========================================
  Files          46       46              
  Lines        4142     4104      -38     
==========================================
- Hits         3415     3380      -35     
+ Misses        727      724       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I've tried to replace as much properties as possible but changing some fields (like in Http) breaks tests as now deserialization of config to sdl outputs more types that were initially provided. That requires either changing tests itself or how identity is compared

Identity tests are required because there are use cases where we generate a new config using open API or Adobe other format. Some times we want to merge two config into one. I don't think we should change it.

Identity tests are required because there are use cases where we generate a new config using open API or Adobe other format. Some times we want to merge two config into one. I don't think we should change it.

Could you please point out to some examples or documentation about this to better understand current usage?

Identity tests should stay I agree, but to be able to replace Option to default we could:

• leave that pr as is with replaced entries that do not affect actual output (like in Field) while leave other entries as is (that are related to directives)
• improve identity tests that they are validate that output == input + defaults and not just input == output

Think about Config as a reduced form of input from the user. The user (a human) can write a Json, Yaml, GraphQL or use a GUI to create an input to tailcall. Let's say the input is of type "I". Now here is what happens next:

• Convert "I" into a consistent input format which in our case is called as "Config". All inputs should be converted into a "Config".
• From Config, we convert to Blueprint and then into a GraphQL executor.
• The reason why we need to have a consistent input format — Config, is that we should be able to convert from one input format to another. So a user could write in .json format, but could allow it to be converted into a .graphql format. Or a user could use a GUI to generate .graphql config, download it and start the server.

I hope this makes sense. Let me know if it doesn't, happy to talk more about the architecture and the reason behind it.

Feel free to connect with us on [discord] if you prefer a more real time collaboration :)

The identity tests, guarantee that we always generate the most minimal and compact configuration that's possible. This is one of the reasons why we started with the Option wrapping.

Identity tests are required because there are use cases where we generate a new config using open API or some other format. Some times we want to merge two config into one. I don't think we should change it.

It's not Adobe I meant some

The identity tests, guarantee that we always generate the most minimal and compact configuration that's possible. This is one of the reasons why we started with the Option wrapping.

In case we want to generate as minimal configuration as possible from Config I have a suggestion to use serde's skip_serializing. In that case fields with that attribute won't appear in the output if they have a default value.

Please, look at my latest commit that shows the example of usage.

One possible side effect of this is that some input settings could disappear from the output if they are equal to the default value

I think this is going in the right right direction. We have a few pending Option<Vec<_>>.

I think this is going in the right right direction. We have a few pending Option<Vec<_>>.

Updated other fields that has clear defaults. Please, note the comment why some Option<Vec> wasn't replaced

/tip $50 @meskill

🎉🎈 @meskill has been awarded $50! 🎈🎊