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

docs: custom gateway #5465

Merged
merged 29 commits into from
Dec 5, 2022
Merged

docs: custom gateway #5465

merged 29 commits into from
Dec 5, 2022

Conversation

alaeddine-13
Copy link
Contributor

@alaeddine-13 alaeddine-13 commented Nov 29, 2022

Add documentation for the custom gateway

  • add gateway API docs

Custom gateway docs:

  • implement BaseGateway
  • fastapi gateway
  • document attributes of BaseGateway (port, ports, protocol, protocols)
  • document the GatewayStreamer
  • needed healtcheck endpoints
  • docker custom gateway
  • use a custom gateway
    YAML spec:
  • update Gateway YAML spec
  • make sure everything is runnable also in k8s

@github-actions github-actions bot added size/M area/docs This issue/PR affects the docs area/testing This issue/PR affects testing labels Nov 29, 2022
@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Merging #5465 (0777193) into master (b36e6bd) will decrease coverage by 1.64%.
The diff coverage is n/a.

❗ Current head 0777193 differs from pull request most recent head b769192. Consider uploading reports for the commit b769192 to get more accurate results

@@            Coverage Diff             @@
##           master    #5465      +/-   ##
==========================================
- Coverage   86.24%   84.60%   -1.65%     
==========================================
  Files         101      101              
  Lines        6667     6657      -10     
==========================================
- Hits         5750     5632     -118     
- Misses        917     1025     +108     
Flag Coverage Δ
jina 84.60% <ø> (-1.65%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/serve/runtimes/gateway/composite/gateway.py 33.33% <0.00%> (-66.67%) ⬇️
jina/jaml/parsers/flow/v1.py 76.11% <0.00%> (-23.89%) ⬇️
jina/orchestrate/flow/base.py 82.60% <0.00%> (-6.59%) ⬇️
...a/orchestrate/deployments/config/docker_compose.py 94.60% <0.00%> (-4.42%) ⬇️
jina/orchestrate/deployments/config/helper.py 91.22% <0.00%> (-3.51%) ⬇️
jina/parsers/helper.py 54.21% <0.00%> (-2.41%) ⬇️
jina/clients/helper.py 97.61% <0.00%> (-2.39%) ⬇️
jina/jaml/helper.py 81.75% <0.00%> (-2.19%) ⬇️
jina/serve/gateway.py 93.65% <0.00%> (-1.67%) ⬇️
jina/serve/runtimes/worker/request_handling.py 90.10% <0.00%> (-1.05%) ⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
* Defining your API Gateway interface. You can define your JSON schema or protos,...

Customization is allowed different components:
* Implementing the custom gateway using a `base` gateway class: {class}`~jina.Gateway` or {class}`~jina.serve.runtimes.gateway.http.fastapi.FastAPIBaseGateway`.
Copy link
Member

Choose a reason for hiding this comment

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

Separate the 2 options. Let's focus on explaining BaseGateway and then have another section for FastAPI.

BTW. For FastAPI we could already expose the health check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's already a section called Implementing the custom gateway which is split in 2 parts for each base gateway

docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
Jina Gateway Runtime will instantiate your implemented class, inject runtime arguments and user-defined arguments to it,
run it, send health-checks to it and orchestrate it.

Two Base Gateway classes are provided to allow implementing a custom gateway:
Copy link
Member

Choose a reason for hiding this comment

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

Lets add FastAPI only in a separatr section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's already a separate subsection for fast api

docs/fundamentals/gateway/custom-gateway.md Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
@alaeddine-13 alaeddine-13 marked this pull request as ready for review November 30, 2022 10:21
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved

(gateway-streamer)=
## Calling Executors with {class}`~jina.serve.streamer.GatewayStreamer`
{class}`~jina.serve.streamer.GatewayStreamer` allows you to interface with Executors within the gateway. An instance of
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to explain what this streamer is and what the user needs to do with it: What methods are exposed, what behaviour do they implement, what is the user expected to do, what are inputs outputs, etc. Some of this will be in the relevant docstrings, then I would at least link to those

Copy link
Contributor Author

@alaeddine-13 alaeddine-13 Dec 1, 2022

Choose a reason for hiding this comment

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

what this streamer is and what the user needs to do with it

added more description

What methods are exposed

The user only needs to know about stream_docs. An example is provided

what is the user expected to do, what are inputs outputs

done

Some of this will be in the relevant docstrings, then I would at least link to those

All mentions of gateway streamer already link to the python api (the links only show properly on released doc builds

Copy link
Contributor

@JohannesMessner JohannesMessner left a comment

Choose a reason for hiding this comment

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

Some more suggestions, sorry, I am always a pain on this stuff ;)

docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Show resolved Hide resolved
docs/fundamentals/gateway/custom-gateway.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the area/testing This issue/PR affects testing label Dec 5, 2022
JohannesMessner
JohannesMessner previously approved these changes Dec 5, 2022
Copy link
Contributor

@JohannesMessner JohannesMessner left a comment

Choose a reason for hiding this comment

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

Like it a lot now 👍

@alaeddine-13 alaeddine-13 reopened this Dec 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

📝 Docs are deployed on https://docs-custom-gateway--jina-docs.netlify.app 🎉

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

📝 Docs are deployed on https://docs-custom-gateway--jina-docs.netlify.app 🎉

@JoanFM JoanFM merged commit 212466e into master Dec 5, 2022
@JoanFM JoanFM deleted the docs-custom-gateway branch December 5, 2022 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs This issue/PR affects the docs size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants