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

versioning idea #27

Closed
wants to merge 1 commit into from
Closed

Conversation

ldulcic
Copy link
Member

@ldulcic ldulcic commented Oct 2, 2024

enum OrbMode {
  ORB_MODE_UNSPECIFIED = 0;
  LEGACY = 1;
  SELF_SERVE = 2;
}

enum OrbType {
  ORB_TYPE_UNSPECIFIED = 0;
  PEARL = 1;
  DIAMOND = 2;
}

message OrbMetadata {
  string orb_id = 1;
  OrbType orb_type = 2;
  OrbMode orb_mode = 3;
}
  • OrbMetadata is a replacement for AnnouncingOrbId message.
  • OrbMetadata is the first message Orb sends to the App.
  • App decides how to communicate with the Orb based on OrbMetadata.orb_mode field.
  • Self Serve mode has all of its messages in self_serve.proto file.

Forward compatibility

If we decide to add a new mode, eg. Self Serve v2, we do this:

  • Add new SELF_SERVE_2 option in OrbMode enum
  • Add new self_serve_v2.proto file with corresponding messages for this mode

If app doesn't support a mode it received, it would prompt user to update the app to verify.

Backward compatibility

  • We can add messages in self_serve.proto but only if they won't break the flow in old apps, every breaking change requires a new OrbMode
  • We can only add fields to OrbMetadata message

@ldulcic ldulcic marked this pull request as draft October 2, 2024 13:38
@@ -0,0 +1,13 @@
syntax = "proto3";

package self_serve.app;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFYI putting v1 in the way it was before, is because protobuf linters suggest so. We are using buf which actually follows Google and FB best practices. I think we should stay on this path :/

Also, I think your concern is that we need to bump the version on every change. That's not the case. This v1 is only if we introduce breaking changes. We can add and augment the structs and messages here without changing the versions. The same for the enums. We just need to make sure that our clients are able to handle unknown states.


package self_serve.app;

message AppSelfServeModeMessage {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against this and I'm actually proposing the same here: 9df4605. Let's do it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong commit hash? :D

@ldulcic ldulcic closed this Oct 3, 2024
@ldulcic
Copy link
Member Author

ldulcic commented Oct 3, 2024

@andronat continued this idea in #30

@ldulcic ldulcic deleted the versioning-idea branch October 3, 2024 19:27
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.

2 participants