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

Transition internal structs to Protocol Buffer Schema #440

Closed
yottahmd opened this issue Apr 24, 2023 · 6 comments
Closed

Transition internal structs to Protocol Buffer Schema #440

yottahmd opened this issue Apr 24, 2023 · 6 comments

Comments

@yottahmd
Copy link
Collaborator

yottahmd commented Apr 24, 2023

Considering transitioning our structs for basic information like Dag, Step, and Status to a Protocol Buffer schema. This change is expected to offer several advantages in the future, including:

  • Schema Versioning: Protocol Buffers support built-in schema versioning, making it easier to evolve our data structures over time without breaking existing applications.
  • Simplified Database Integration: By utilizing Protocol Buffers, we can store data of those structure in any storage easily and efficiently compared to JSON.
  • REST API Generation with gRPC Gateway: With Protocol Buffers and gRPC Gateway, we can automatically generate REST APIs from our schema and documents.

I think it is a good stepping stone towards a gRPC-based client-server architecture that will make it easier to implement features to manage remote servers from local machines.

@garunitule
Copy link
Contributor

Hi, @yohamta

I am also in favor of the transition to Protocol Buffer and the gradual transition from Protocol Buffer to gRPC.

May I work on it?

@yottahmd
Copy link
Collaborator Author

yottahmd commented May 3, 2023

Hi @garunitule , that would be really helpful! Please, go ahead and thank you so much for your support!

@garunitule
Copy link
Contributor

@yohamta
May I discuss one thing with you?

I would like to discuss how to handle the OutputVariables field (*utils.SyncMap type) in the Step type.

I could only think of wrapping the type generated by protocol buffers and adding an OutputVariables field.
Is there any other way? From an operational standpoint, I would like to use the generated type as is.

@yottahmd
Copy link
Collaborator Author

yottahmd commented May 7, 2023

Hi @garunitule 👋

Thank you for working on this!

I suggest changing the OutputVariables type to a regular map and using a sync.Mutex to lock the sections where the values are being read or modified. We can add a sync.Mutex field to the Node struct, which acts like a wrapper for Step. Also, we might need to make some minor adjustments to the related sections.

Is there any other way? From an operational standpoint, I would like to use the generated type as is.

Agreed 🙂

@garunitule
Copy link
Contributor

@yohamta
Thank you for your response!
I will proceed with making some minor adjustments.

@yottahmd
Copy link
Collaborator Author

Sorry, let me close this issue for now. We’ll keep JSON format for now to prioritize feature implementation to go to cloud.

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

No branches or pull requests

2 participants