search

jina-ai / jina

☁️ Build multimodal AI applications with cloud-native stack
https://docs.jina.ai
Apache License 2.0
20.63k stars 2.21k forks source link

Best Way to Pass Request Headers to Executor #6049

Closed NarekA closed 2 months ago

NarekA commented 10 months ago

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

JoanFM commented 10 months ago

Hey @NarekA,

This is the only way for the moment.

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

NarekA commented 10 months ago

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.

JoanFM commented 10 months ago

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?

NarekA commented 10 months ago

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.

JoanFM commented 10 months ago

Makes sense,

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

Thank you very much for the proposal

NarekA commented 9 months ago

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

JoanFM commented 9 months ago

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

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

NarekA commented 9 months ago

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.

JoanFM commented 9 months ago

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.

NarekA commented 9 months ago

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.

JoanFM commented 9 months ago

yes this looks reasonable.

dkmiller commented 9 months ago

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?

JoanFM commented 9 months ago

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.

NarekA commented 9 months ago

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.

jina-bot commented 6 months ago

@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 commented 3 months ago

@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