GeezerVison

For targets, actor, and metadata, couldn't these use the schemas AuditLogSchemaTarget, AuditLogSchemaActor, and AuditLogSchemaMetadata, rather than the open-ended mapping of str: Any? (Note that these params are reused in other places in this PR, too, I just picked one example.)

So, my initial thinking when implementing this that these arguments were exactly as flexible as the types indicated (e.g. action is literally an arbitrary string, metadata is an arbitrary mapping). Leaving them this way enabled folks to write bog-standard/flexible Python code like:

workos.audit_logs.create_schema(
    action="document.updated",
    targets=[
        {
            "type": "document",
            "metadata": {"title": "string"},
        }
    ],
    actor={"metadata": {"role": "string"}},
    metadata={"invoice_id": "string"},
)

This did lack type safety, but it felt like a type alias was un-necessary redirection (and created the false impression of type safety), and using the direct model (e.g. AuditLogSchemaTarget) would mean folks need to both import those models and then construct the call like so:

from workos.types.audit_logs import (
    AuditLogSchemaTarget,
    AuditLogSchemaActor,
    AuditLogSchemaMetadata,
    AuditLogSchemaMetadataProperty,
)

workos.audit_logs.create_schema(
    action="document.updated",
    targets=[
        AuditLogSchemaTarget(
            type="document",
            metadata=AuditLogSchemaMetadata(
                type="object",
                properties={
                    "title": AuditLogSchemaMetadataProperty(type="string")
                },
            ),
        )
    ],
    actor=AuditLogSchemaActor(
        metadata=AuditLogSchemaMetadata(
            type="object",
            properties={
                "role": AuditLogSchemaMetadataProperty(type="string")
            },
        )
    ),
)

I think you'll agree, this feels bad, especially compared with the Node/Typescript implementation.

Having said that, your comment (and the subsequent digging it triggered) reminded me that I can actually provide just a little extra help via a TypedDict, which led me to create types like:

class AuditLogSchemaTargetInput(TypedDict):
    type: str
    metadata: NotRequired[MetadataSchemaInput]

class AuditLogSchemaActorInput(TypedDict):
    metadata: MetadataSchemaInput

Which means that now folks get value suggestions in their IDE and also validation that they're constructing the mappings correctly, while still passing normal mappings in:

workos.audit_logs.create_schema(
    action="document.updated",
    targets=[
        {
            "type": "document",
            "metadata": {"title": "string"},
        }
    ],
    actor={"metadata": {"role": "string"}},
    metadata={"invoice_id": "string"},
)

I think this ultimately provides the balance of ergonomics and safety we want.

sick 🤙

(Note that these params are reused in other places in this PR, too, I just picked one example.)

Almost forgot to comment on this as well. The other functions I examined didn't, in practice, benefit from the kind of type safety the create_schema endpoint specifically did. But I'm happy to dig deeper there if you have feelings (or just think we should have type aliases in general).

Given these changes, and the fact that that warning will be removed,

As of v2.12, this warning was removed! Do you think we can update our pydantic dependency (from 2.10.4, bleh) and remove this warning? Or does updating our dependency affect downstream users (necessitating a major version bump)?

does updating our dependency affect downstream users (necessitating a major version bump)?

Imo, bumping up the minimum Pydantic version wouldn't, on the face of it, require a major SDK version bump, but given the popularity of Pydantic, it's possible that it could break downstream users who are pinned to a lower version.

My 2¢ is that we should leave it for now and then revisit a version bump later when we're dropping support for EOL Python versions.

That said, I'm happy to make that change in a separate PR if you feel strongly about it.

Yeah, let's roll it into the next major bump.