Automatically deriving Jason.Encoder

[feynmanliang]

When an HTTP client (Tesla+Finch in our case) uses Jason to SerDe HTTP bodies, lack of a Jason.Encoder protocol implementation results in the following error:

[warning] Error: {:error, {Tesla.Middleware.JSON, :encode, %Protocol.UndefinedError{protocol: Jason.Encoder, value: %OpenAI.ChatCompletionRequestMessage{...}, description: "Jason.Encoder protocol must always be explicitly implemented.\n\nIf you own the struct, you can derive the implementation specifying which fields should be encoded to JSON:\n\n    @derive {Jason.Encoder, only: [....]}\n    defstruct ...\n\nIt is also possible to encode all fields, although this should be used carefully to avoid accidentally leaking private information when new fields are added:\n\n    @derive Jason.Encoder\n    defstruct ...\n\nFinally, if you don't own the struct you want to encode to JSON, you may use Protocol.derive/3 placed outside of any module:\n\n    Protocol.derive(Jason.Encoder, NameOfTheStruct, only: [...])\n    Protocol.derive(Jason.Encoder, NameOfTheStruct)\n"}}}

For Tesla, one solution is to just use maps rather than the generated structs but this sacrifices type checking.

Our current workaround is to derive these encoders in clients importing the autogenerated libraries i.e.

require Protocol
Protocol.derive(Jason.Encoder, NameOfTheStruct)

Instead, it would be better if the autogenerated libraries worked out of the box without additional end-user effort.

Question: Is there an easy way to modify code generation to insert a [derive](require Protocol
Protocol.derive(Jason.Encoder, TextGenerationInference.GenerateResponse)) (I think @derive {Jason.Encoder} would suffice for our use case, as everything published in OpenAPI specifications is public right?) before all defstructs emitted by open-api-generator?

I realize that the client is left abstract and not everyone is using Jason for their JSON needs. So maybe this should be opt in.

[aj-foster]

Here's a possible solution:

What if we add a setting like schema_use where library authors can specify a module that should be called with the use macro at the top of each schema? Then, you could implement anything you want in the body of the macro, including @derive statements.

[feynmanliang]

Thanks for looking at this issue!

That could work if we passed in module since we need the module name to derive the Encoder protocol for. Any other per-schema things library authors might want?

[aj-foster]

Commit c33d617 added a new configuration option, schema_use, as described above. I thought about your feedback around using the module name, and luckily this is possible using the __MODULE__ value from the macro environment. In the docs I left it open that there may be options passed in the future, but none right now. If we think of anything, we can always add it.

With it, you can do something like this:

# with schema_use: MyModule

defmodule MyModule do
  defmacro __using__(_opts) do
    quote do
      @derive {Jason.Encoder, except: [:__info__]}
    end
  end
end

(In this example, __info__ is part of the extra_fields configuration that shouldn't be encoded.)

This could be used with conditional logic, for example if Code.ensure_loaded?(Jason) do, if your client library allows people to bring-their-own deserializer.

[feynmanliang]

Amazing! Thank you for the quick follow up. Will give it a shot on tgi.ex this week and report back.

…

On Sun, May 28, 2023 at 12:49 AJ Foster ***@***.***> wrote: Commit c33d617 <c33d617> added a new configuration option, schema_use, as described above. I thought about your feedback around using the module name, and luckily this is possible using the __MODULE__ value from the macro environment. In the docs I left it open that there *may* be options passed in the future, but none right now. If we think of anything, we can always add it. With it, you can do something like this: # with schema_use: MyModule defmodule MyModule do defmacro __using__(_opts) do quote do @derive {Jason.Encoder, except: [:__info__]} end endend (In this example, __info__ is part of the extra_fields configuration that shouldn't be encoded. — Reply to this email directly, view it on GitHub <#11 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHRW5KR6FGSMPPAIRUHLI3XIOT3TANCNFSM6AAAAAAYIOICBQ> . You are receiving this because you authored the thread.Message ID: ***@***.*** com>

[feynmanliang]

This resolves moving the Encoder derivation into the library, thank you @aj-foster.

• For some reason, OpenAI got downcased to OpenAi (https://github.com/fmops/oapi_oai/pull/2/files#diff-8b18aa85ed1df79da09e1ba243d2b51be958cbc3c368d1c4f83857ba580f248eL1) after bumping up from [email protected] (https://github.com/fmops/oapi_oai/pull/2/files#diff-dfa6f4ed74c90e5d4fda283d547d366586e690387289bcfd473e3fa5f9ace308L30).
  • Since the change is backwards incompatible (https://github.com/fmops/oapi_oai/actions/runs/5124550889/jobs/9216529373?pr=2) and rename in Config seems to only support schemas and not module names, I've manually patched it (fmops/oapi_oai@7ea088a) for now.
  • Possibly related to cbaf0ce?
• I still need to convert non-required field's nils into missing fields (https://github.com/fmops/oapi_oai/pull/2/files#diff-6b5aa81ddf4f8d94a4acc89038b71eac262c3e7c12aebf5d6803b27f9fa3a012R8), but let's save this discussion for Nullable vs. Non-Required Fields in Typespecs #7

[aj-foster]

I haven't had time to look into the name change, but you may also want to keep an eye on #14 which gives more flexibility in the naming of operation modules. Not sure if it's fully applicable to your case, but it may have an impact.

[feynmanliang]

Ran a bisect, cbaf0ce is the offending commit. Will close this discussion and create an issue for it