jina icon indicating copy to clipboard operation
jina copied to clipboard

Best Way to Pass Request Headers to Executor

Open NarekA opened this issue 1 year ago • 16 comments

Describe your proposal/problem

I've been looking for a way to pass http request headers to my Executor, and this is the only way I've been able to do it.


class MyGateway(FastAPIBaseGateway):
    @property
    def app(self):
        from fastapi import FastAPI

        app = FastAPI(title="Test Gateway")

        @app.get(path="/config")
        async def custom_endpoint(my_input: MyInput, request: Request):
            docs1: DocList[RequestOutput] = await self.executor["executor0"].post(
                on="/api/v0/test",
                inputs=DocList[Input]([my_input]),
                parameters=request.headers or {"no": "headers"},
                return_type=DocList[RequestOutput],
            )
            return docs1.to_json()

        return app

Is there a simpler way to do this? The approach above only works for flows, is there a way that works for deployments?


Environment

  • jina 3.20.3
  • docarray 0.33.0
  • jcloud 0.2.16
  • jina-hubble-sdk 0.39.0
  • jina-proto 0.1.27
  • protobuf 3.20.0
  • proto-backend python
  • grpcio 1.48.0rc1
  • pyyaml 6.0.1
  • python 3.9.17
  • platform Darwin
  • platform-release 22.6.0
  • platform-version Darwin Kernel Version 22.6.0: Wed Jul 5 22:21:53 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6020
  • architecture arm64
  • processor arm
  • uid 11592019587001
  • session-id 0062ff1e-574c-11ee-9138-0a8afa35afb9
  • uptime 2023-09-19T17:24:02.674004
  • ci-vendor (unset)
  • internal False
  • JINA_DEFAULT_HOST (unset)
  • JINA_DEFAULT_TIMEOUT_CTRL (unset)
  • JINA_DEPLOYMENT_NAME (unset)
  • JINA_DISABLE_UVLOOP (unset)
  • JINA_EARLY_STOP (unset)
  • JINA_FULL_CLI (unset)
  • JINA_GATEWAY_IMAGE (unset)
  • JINA_GRPC_RECV_BYTES (unset)
  • JINA_GRPC_SEND_BYTES (unset)
  • JINA_HUB_NO_IMAGE_REBUILD (unset)
  • JINA_LOG_CONFIG (unset)
  • JINA_LOG_LEVEL (unset)
  • JINA_LOG_NO_COLOR (unset)
  • JINA_MP_START_METHOD (unset)
  • JINA_OPTOUT_TELEMETRY (unset)
  • JINA_RANDOM_PORT_MAX (unset)
  • JINA_RANDOM_PORT_MIN (unset)
  • JINA_LOCKS_ROOT (unset)
  • JINA_K8S_ACCESS_MODES (unset)
  • JINA_K8S_STORAGE_CLASS_NAME (unset)
  • JINA_K8S_STORAGE_CAPACITY (unset)
  • JINA_STREAMER_ARGS (unset)

NarekA avatar Sep 20 '23 00:09 NarekA

Hey @NarekA,

This is the only way for the moment.

May I ask what is the intended usage you want for this feature?

JoanFM avatar Sep 20 '23 07:09 JoanFM

Our service hits multiple remote deployments of the same model, and we want the ability to use fields in the request headers to determine which deployment the user hits. We can use parameters, but those are easier to spoof and are not auto-populated. In addition, there are some observability data in our headers which would be useful.

NarekA avatar Sep 21 '23 15:09 NarekA

Right now we do not have this chance.

It would be interesting to understand how you would imagine this happening for you?

How would u expect this to work?

Would u get them in the Executor? Regardless of the protocol? how would the Jina Client work with them?

JoanFM avatar Sep 21 '23 16:09 JoanFM

It would be nice if we could just set argument request: Request = None in our executor, and the HTTP Executor would automatically pass it. For other executors it would just default to None.

For the client, we could add an argument for headers, but even if we don't it would still be helpful since the headers we need are set by our network proxy. Other headers could be set by using an http client instead of the Jina client.

Alternatively, we could copy the flask approach and create a request context:

from flask import Flask, request

@app.route('/query-example')
def query_example():
    # if key doesn't exist, returns None
    language = request.args.get('language')

We need the request context for the same reasons any web application might need request context, building a gateway proxy just to get access to basic request parameters seems like overkill.

NarekA avatar Sep 21 '23 23:09 NarekA

Makes sense,

I will try to work on it when I get the time.

Thank you very much for the proposal

JoanFM avatar Sep 22 '23 07:09 JoanFM

@JoanFM I might be able to give this a shot. I assume most of the changes would be in the FastAPIBaseServer and BaseExecutor classes?

NarekA avatar Sep 26 '23 16:09 NarekA

yes, but I would like to first discuss the user experience.

Also it should be something u could enjoy from Grpc serving as metadata.

JoanFM avatar Sep 26 '23 16:09 JoanFM

That's a good idea, do we want to?

  1. pass the request object for http requests and the context object for GRPC or:
  2. Limit the scope to headers and invocation_metadata.

I like the first option because it will give users additional metadata such as user-agent, but the 2 objects have different interfaces so it might be a good idea to use a different parameter names for them or normalize them.

NarekA avatar Sep 26 '23 16:09 NarekA

I would limit the scope to headers, otherwise user would expect in Requests to get all the data, and this is something I want to hide, because I think is still good to simply get docarray in and out as the basic operatio s.

JoanFM avatar Sep 26 '23 16:09 JoanFM

That makes it easier as both headers and GRPC metadata can be passed in as dictionaries. We can have an optional argument called metadata which would follow the same pattern as parameters where it is only passed in when the function signature contains the keyword argument and defaults to None.

NarekA avatar Sep 26 '23 17:09 NarekA

yes this looks reasonable.

JoanFM avatar Sep 26 '23 17:09 JoanFM

Looks like we're moving towards consensus (headers and gRPC metadata in a unified metadata parameter)?

What will it take to get this into the framework?

dkmiller avatar Oct 17 '23 21:10 dkmiller

Hey @dkmiller,

It would not be so complicated, but currently we have not so much time to work on it. it would be nice if @NarekA or you @dkmiller could contribute this feature.

JoanFM avatar Oct 18 '23 02:10 JoanFM

I will see if I can get this prioritized, we are currently using a workaround (of passing header values as inputs) but it's not ideal.

NarekA avatar Oct 18 '23 14:10 NarekA

@jina-ai/product This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 14 days

jina-bot avatar Jan 17 '24 00:01 jina-bot

@jina-ai/product This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 14 days

jina-bot avatar Apr 17 '24 00:04 jina-bot