flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.41k stars 580 forks source link

[Housekeeping] Support protoc-gen-doc to generate json tags with camel case #2120

Open pmahindrakar-oss opened 2 years ago

pmahindrakar-oss commented 2 years ago

Describe the issue

Current version of protoc-gen-doc being used generates the snake case version of the fields for json marshalling

type ClusterConfig struct {
    state         protoimpl.MessageState
    sizeCache     protoimpl.SizeCache
    unknownFields protoimpl.UnknownFields

    // List of plugins that this cluster can run.
    EnabledPlugins []string `protobuf:"bytes,1,rep,name=enabled_plugins,json=enabledPlugins,proto3" json:"enabled_plugins,omitempty"`
    // List of projects for which this cluster can schedule executions.
    EnabledProjects []string `protobuf:"bytes,2,rep,name=enabled_projects,json=enabledProjects,proto3" json:"enabled_projects,omitempty"`
}

Would be great if this behavior is customizable.

Currently the flytectl uses camelcase for all its commandline flags In some cases where we want to allow users to read/write such configs from json/yaml files and pass them to cli, then in the existing behavior it results in empty attributes due to this diff.

eg : while defining the commandline flag counterpart for the above ClusterConfig we define this way to match up with the json tag in the generated definition of ClusterConfig

type UpdateClusterConfig struct {
    DryRun            bool   `json:"dryRun" pflag:",execute command without making any modifications."`
    ClusterConfigFile string `json:"clusterConfigFile" pflag:",file containing the update cluster config parameters."`
    EnabledPlugins    []string `json:"enabled_plugins" pflag:",comma separated list of plugins to be enabled on the cluster."`
    EnabledProjects   []string `json:"enabled_projects" pflag:",comma separated list of projects to be enabled on the cluster."`
}

Expected to keep the cli config using camel case and still be able to use ClusterConfig to write a yaml file in camelCase

type UpdateClusterConfig struct {
    DryRun            bool   `json:"dryRun" pflag:",execute command without making any modifications."`
    ClusterConfigFile string `json:"clusterConfigFile" pflag:",file containing the update cluster config parameters."`
    EnabledPlugins    []string `json:"enabledPlugins" pflag:",comma separated list of plugins to be enabled on the cluster."`
    EnabledProjects   []string `json:"enabledProjects" pflag:",comma separated list of projects to be enabled on the cluster."`
}

What if we do not do this?

mixture of camel case and snake case or add customized unmarshalling

Related component(s)

No response

Are you sure this issue hasn't been raised already?

Have you read the Code of Conduct?

pmahindrakar-oss commented 2 years ago

Using JsonPBMarshaller for now

func WriteClusterConfigToFile(clusterConfig *cluster.ClusterConfig, fileName string) error {
    JsonPbMarshaller := &jsonpb.Marshaler{}
    str ,_ := JsonPbMarshaller.MarshalToString(clusterConfig)
    d, err := yaml.JSONToYAML([]byte(str))
github-actions[bot] commented 1 year ago

Hello πŸ‘‹, This issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will close the issue if we detect no activity in the next 7 days. Thank you for your contribution and understanding! πŸ™

github-actions[bot] commented 1 year ago

Hello πŸ‘‹, This issue has been inactive for over 9 months and hasn't received any updates since it was marked as stale. We'll be closing this issue for now, but if you believe this issue is still relevant, please feel free to reopen it. Thank you for your contribution and understanding! πŸ™

github-actions[bot] commented 1 month ago

Hello πŸ‘‹, this issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will engage on it to decide if it is still applicable. Thank you for your contribution and understanding! πŸ™