Skip to content
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

policy: erase type to any #166

Closed
wants to merge 2 commits into from
Closed

Conversation

jrobsonchase
Copy link
Contributor

@jrobsonchase jrobsonchase commented Jun 11, 2024

We're probably going to be making more changes to the schema of this. Guard against having to release new SDKs all the time whenever we do so, and defer validation to the server side.

Note: this is currently a breaking change - probably want to make it backwards-compatible.
OK, backwards-compatible now! any is better than map[string]any for this.

@jrobsonchase jrobsonchase force-pushed the josh/type-erase-policy branch from 2d032b9 to 5b0aa96 Compare June 11, 2024 16:05
@jrobsonchase jrobsonchase changed the title policy: erase type to map[string]any policy: erase type to any Jun 11, 2024
@jrobsonchase jrobsonchase force-pushed the josh/type-erase-policy branch from 5b0aa96 to a355c50 Compare June 11, 2024 16:06
@jrobsonchase jrobsonchase force-pushed the josh/type-erase-policy branch from a355c50 to c2784a0 Compare June 11, 2024 16:15
@jrobsonchase
Copy link
Contributor Author

Hmmmm, this passes tests, but the Action.Config type has changed on the wire, so this won't actually work. Will need server-side support to fully support this new approach.

@TheConcierge
Copy link
Contributor

hey, Josh. I like the idea here! In fact, there is an open RFC that attempts to address this exact concern of having to constantly update the SDKs. I went down the route of changing the type of the protobuf (see section in RFC), but it's difficult considering we are working with a deeply nested proto that we can't easily change the behavior of.
The idea of the RFC here is that this just becomes a string over the wire so there are no protobuf changes we need to propagate to the SDKs moving forward, everything is done server-side

@CK-Ward
Copy link
Contributor

CK-Ward commented Jun 11, 2024

Closing this per today's discussion. @nijikokun @tammybailey

@CK-Ward CK-Ward closed this Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants