Add typing for Directory Sync events #282

ameesha · 2024-07-02T11:26:09Z

Description

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

workos/event_objects/directory.py

mattgd · 2024-07-02T13:55:57Z

workos/event_objects/directory.py

+class DirectoryType(Enum):
+    AZURE_SCIM_v2 = "azure scim v2.0"
+    BAMBOO_HR = "bamboohr"
+    BREATHE_HR = "breathe hr"
+    CEZANNE_HR = "cezanne hr"
+    CYBERARK_SCIM_v2 = "cyperark scim v2.0"
+    FOURTH_HR = "fourth hr"
+    GENERIC_SCIM_v2 = "generic scim v2.0"
+    GOOGLE = "gsuite directory"
+    GUSTO = "gusto"
+    HIBOB = "hibob"
+    JUMPCLOUD_SCIM_v2 = "jump cloud scim v2.0"
+    OKTA_SCIM_v2 = "okta scim v2.0"
+    ONELOGIN_SCIM_v2 = "onelogin scim v2.0"
+    PEOPLE_HR = "people hr"
+    PERSONIO = "personio"
+    PINGFEDERATE_SCIM_v2 = "pingfederate scim v2.0"
+    RIPPLING_SCIM_v2 = "rippling v2.0"
+    SFTP = "sftp"
+    SFTP_WORKDAY = "sftp workday"
+    WORKDAY = "workday"


Do we need S3 or plain Rippling here, or are those totally deprecated?

yeah S3, Rippling (non-SCIM) and Gusto are completely deprecated

Got it, so we can remove Gusto from this list also.

According to mode, these are the directory types that actually exist in produiction:

We don't actually have any Personio directories, which is no big deal.

But we do have Rippling directories (non-SCIM) in the DB. That's not reflected here.

We don't actually have any "untyped" directories in prod anymore, so we don't need to worry about this pending state: https://github.com/workos/workos/blob/1b4c7a6a83b49ec3a605ff4f2f5dd17848e6085d/packages/api/src/directories/directories.controller.ts#L63

We should have a KTLO item to clean that up.

This was for the events list so we wouldn't have events with these types anymore.

Nvm catching up to see we're doing API as well now lol

workos/event_objects/directory_user.py

workos/events.py

PavanKulkarni · 2024-07-02T15:08:37Z

workos/event_objects/directory.py

+
+
+class DirectoryState(Enum):
+    ACTIVE = "active"


we never show validating right?

I don't believe there's ever an directory event emitted with status = validating (or any status besides active, deleting).

I don't see where we exclude validating directories: https://github.com/workos/workos/blob/1b4c7a6a83b49ec3a605ff4f2f5dd17848e6085d/packages/api-services/src/directories/directories.service.ts#L322

Via the directory API we definitely return other directory states, but the type is specifically event-scoped where we'd only have active or deleting directories. If we want to enumerate all states, we could extract this enum from the event context.

So, is the idea that we are only typing the events and not the dsync APIs?

Ideally we should type all the things, but I think that's worth a separate PR.

Sure. But I'm don't think we should release the SDK until both the Dsync events and API resources are typed.

Yep, agreed.

Ended up adding some light typing for the APIs using existing objects. The list endpoints could use some better typing, but would require a size-able rework of the auto_paging_iter to do so.

tribble · 2024-07-02T16:55:31Z

workos/event_objects/directory.py

+        self.event: str = attributes["event"]
+        self.id: str = attributes["id"]
+        self.created_at = attributes["created_at"]
+        self.data: DirectoryEvent = DirectoryEvent(attributes["data"])


We can't use this same type here. The directory deleted event is not the same shape as the directory activated event: https://workos.com/docs/events/directory-sync

It uses the directory type, not the legacy directory type with extra fields: https://github.com/workos/workos/blob/e95130bf78e01b202c7e7417adf36135cfc5f033/packages/api-json-models/src/events/directory-deleted.ts#L9

Good catch, I'll update. Is DirectoryWithLegacyFields still an accurate descriptor of the object we use for the other events, or is it here to stay?

Per the events JSON models, it is accurate.

As for "here to stay", I think it contains fields that are no longer necessary. But I'm dubious about the dsync team prioritizing an effort to remove the unnecessary fields. So, likely here to stay for a while.

tribble · 2024-07-02T17:14:33Z

workos/event_objects/directory_group.py

+        self.organization_id: str = attributes["organization_id"]
+        self.created_at: str = attributes["created_at"]
+        self.updated_at: str = attributes["updated_at"]
+        self.raw_attributes: JsonDict = attributes.get("raw_attributes", {})


@PavanKulkarni didn't we stop emitting raw_attributes for directory group events?

Let me confirm this in LD.

yes only 3 teams still depend on the flag. That being said we still send raw_attributes IIRC, it's just empty.

tribble · 2024-07-02T17:55:59Z

workos/event_objects/directory_user.py

+        self.raw_attributes: JsonDict = attributes.get("raw_attributes", {})
+        self.created_at: str = attributes["created_at"]
+        self.updated_at: str = attributes["updated_at"]
+        self.role: Optional[DirectoryUserRole] = (


Is this actually optional anymore?

I didn't think so, but it's still optional in the API event schema, so will keep as-is until the API is updated.

It's the API resource that led me to ask this question. The role here always exists when returning a directory user: https://github.com/workos/workos/blob/e95130bf78e01b202c7e7417adf36135cfc5f033/packages/api/src/directory-users/directory-users.controller.ts#L340

It's optional! We don't return it on non-scim/Google users

https://github.com/workos/workos/blob/main/packages/api-services/src/directory-users/utils/serialize-directory-user.ts#L79

@ameesha but in the code I referenced role always exists because getRoleFromUserRoleId returns Promise<Role>.

I see that the serializer accepts role?, but in this code path I don't see how it can ever actually happen.

yeah the role is always grabbed but in the serializer (line i sent), we only use it for scim/google dirs

nothing cares about whether the role is relevant in the directory type or not but the serializer. it's the only* thing that checks that logic (cause we create and set roles across the board).

`* there are other checks for directory type so we can end recalculations early but if we removed it, nothing would really change

workos/directory_sync.py

ameesha · 2024-07-08T11:49:16Z

workos/events.py

@@ -78,4 +121,11 @@ def list_events(
            "method": Events.list_events,
        }

+        data = []
+        for list_data in response["data"]:
+            if list_data["event"] in EVENT_TO_OBJECT.keys():


if we add a new event or update an event type, until the SDK is explicitly updated we'll just ignore it

add typing for dsync events

9b4f7a9

ameesha requested a review from a team as a code owner July 2, 2024 11:26

ameesha requested a review from PaulAsjes July 2, 2024 11:26

ameesha added 7 commits July 2, 2024 13:39

add directory group and directory user

692b5cb

undo api change

44826fa

fmt

75578f4

ts -> py

622036f

fmt

49bb7ba

new name

7e98632

rename files

d6a137f

ameesha commented Jul 2, 2024

View reviewed changes

workos/event_objects/directory.py Outdated Show resolved Hide resolved

ameesha added 6 commits July 2, 2024 14:22

export type util

548dc53

save

1565d73

static event names

5d88c9f

oops

3ce4d32

fmt

88f0ec1

change typing for py 3.8to10

b5c2a6b

mattgd reviewed Jul 2, 2024

View reviewed changes

PavanKulkarni reviewed Jul 2, 2024

View reviewed changes

tribble reviewed Jul 2, 2024

View reviewed changes

Updates.

bd9b1f4

mattgd self-assigned this Jul 2, 2024

mattgd requested review from tribble and PavanKulkarni July 2, 2024 20:26

Improve directory API typing.

6c14e8a

tribble reviewed Jul 3, 2024

View reviewed changes

workos/directory_sync.py Outdated Show resolved Hide resolved

mattgd and others added 2 commits July 3, 2024 13:26

Update typing.

bd8020c

dupe method and typing

3fea3d1

remove separate dsync event list

6025c77

ameesha commented Jul 8, 2024

View reviewed changes

ameesha added 2 commits July 9, 2024 13:54

auth and connection event types

e8bb412

fmt

7d82288

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typing for Directory Sync events #282

Add typing for Directory Sync events #282

ameesha commented Jul 2, 2024

mattgd Jul 2, 2024

PavanKulkarni Jul 2, 2024 •

edited

Loading

mattgd Jul 2, 2024

tribble Jul 2, 2024

ameesha Jul 8, 2024

ameesha Jul 8, 2024

PavanKulkarni Jul 2, 2024

mattgd Jul 2, 2024

ameesha Jul 2, 2024

tribble Jul 2, 2024

mattgd Jul 2, 2024

tribble Jul 2, 2024

mattgd Jul 2, 2024

tribble Jul 2, 2024

mattgd Jul 3, 2024

mattgd Jul 3, 2024

tribble Jul 2, 2024

mattgd Jul 2, 2024

tribble Jul 2, 2024

tribble Jul 2, 2024

PavanKulkarni Jul 2, 2024

PavanKulkarni Jul 2, 2024

tribble Jul 2, 2024

mattgd Jul 2, 2024

tribble Jul 2, 2024

ameesha Jul 8, 2024

ameesha Jul 8, 2024

tribble Jul 8, 2024

ameesha Jul 8, 2024

ameesha Jul 8, 2024 •

edited

Loading

ameesha Jul 8, 2024

Add typing for Directory Sync events #282

Are you sure you want to change the base?

Add typing for Directory Sync events #282

Conversation

ameesha commented Jul 2, 2024

Description

Documentation

Choose a reason for hiding this comment

PavanKulkarni Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ameesha Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PavanKulkarni Jul 2, 2024 •

edited

Loading

ameesha Jul 8, 2024 •

edited

Loading