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

Best Way to Pass Request Headers to Executor #6049

Closed
NarekA opened this issue Sep 20, 2023 · 16 comments · May be fixed by #6126
Closed

Best Way to Pass Request Headers to Executor #6049

NarekA opened this issue Sep 20, 2023 · 16 comments · May be fixed by #6126
Labels

Comments

@NarekA
Copy link
Contributor

NarekA commented Sep 20, 2023

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)
@JoanFM
Copy link
Member

JoanFM commented Sep 20, 2023

Hey @NarekA,

This is the only way for the moment.

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

@NarekA
Copy link
Contributor Author

NarekA commented Sep 21, 2023

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
Copy link
Member

JoanFM commented Sep 21, 2023

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
Copy link
Contributor Author

NarekA commented Sep 21, 2023

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
Copy link
Member

JoanFM commented Sep 22, 2023

Makes sense,

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

Thank you very much for the proposal

@NarekA
Copy link
Contributor Author

NarekA commented Sep 26, 2023

@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
Copy link
Member

JoanFM commented Sep 26, 2023

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
Copy link
Contributor Author

NarekA commented Sep 26, 2023

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
Copy link
Member

JoanFM commented Sep 26, 2023

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
Copy link
Contributor Author

NarekA commented Sep 26, 2023

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
Copy link
Member

JoanFM commented Sep 26, 2023

yes this looks reasonable.

@dkmiller
Copy link

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
Copy link
Member

JoanFM commented Oct 18, 2023

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
Copy link
Contributor Author

NarekA commented Oct 18, 2023

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
Copy link
Member

@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 jina-bot added the Stale label Jan 17, 2024
@JoanFM JoanFM removed the Stale label Jan 17, 2024
@jina-bot
Copy link
Member

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants