flyteorg / flyte

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

[Docs] Separate Flyte Admin Service.proto into multiple files #698

Open kumare3 opened 3 years ago

kumare3 commented 3 years ago

Motivation: Why do you think this is important? Currently the entire service API specification is one file. It would be much better to split it into multiple files, as this with automatically separate the generated documentation here

More than documentation, it is also easier to reason about the service and eventually if required decompose into multiple processes (though this is not a strict requirement)

Goal: What should the final outcome look like, ideally? Separate documents for execution_service.proto workflow_service.proto task_service.proto etc

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Flyte component

kumare3 commented 3 years ago

On looking more carefully, I think the documentation itself is not being generated for the service. It might be better to use an off-the shelf generator like https://github.com/pseudomuto/protoc-gen-doc

Here is an example command to generate using this

docker run --rm   -v $(pwd)/gen/:/out   -v $(pwd)/protos:/protos   pseudomuto/protoc-gen-doc -I=/usr/local/include -I=/opt/include -I=./protos -I=flyteidl/admin flyteidl/admin/common.proto flyteidl/admin/event.proto flyteidl/admin/execution.proto flyteidl/admin/launch_plan.proto flyteidl/admin/matchable_resource.proto flyteidl/admin/node_execution.proto flyteidl/admin/notification.proto flyteidl/admin/project.proto flyteidl/admin/project_domain_attributes.proto flyteidl/admin/schedule.proto flyteidl/admin/task.proto flyteidl/admin/task_execution.proto flyteidl/admin/workflow.proto flyteidl/admin/workflow_attributes.proto

You can generate per proto-file and generate a markdown output.

kumare3 commented 3 years ago

cc @cosmicBboy / @samhita-alla / @SandraGH5 / @pmahindrakar-oss

wild-endeavor commented 3 years ago

This will require a change to FlyteAdmin as well.

kumare3 commented 3 years ago

This will require a change to FlyteAdmin as well.

Flytectl, flytekit and probably flytekit-java

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! πŸ™

kumare3 commented 9 months ago

this is being revived and may be scoped down. cc @katrogan

kumare3 commented 9 months ago

we will probably only move event service out

github-actions[bot] commented 1 week 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! πŸ™