add index name header to all requests #272
Conversation
vicilliar
had a problem deploying
to
cloud-tests
GitHub Actions
Failure
vicilliar
had a problem deploying
to
cloud-tests
GitHub Actions
Failure
|
Cloud integration tests: https://github.com/marqo-ai/py-marqo/actions/runs/13004448466/job/36268777672 Only 2 tests failing are due to cloud staging not being on 2.15 yet. |
| @@ -57,6 +56,7 @@ def send_request( | |||
| index_name: str = "" | |||
| ) -> Any: | |||
| req_headers = copy.deepcopy(self.headers) | |||
| req_headers[CLOUD_INDEX_NAME_HTTP_HEADER] = index_name | |||
There was a problem hiding this comment.
What should we do if index_name == ""?
I'm pretty confident that it won't break anything but should we add this header in the first place if index_name argument isn't passed?
There was a problem hiding this comment.
Actually I'd prefer if we don't send any index name header at all in this case. This will be for requests like mq.get_indexes(). Do those need the index name header in gcp?
There was a problem hiding this comment.
It depends, if the client was instantiated with the marqo cloud api endpoint, it is not needed, however if it is instantiated with the index endpoint directly, then yes it is required.
There was a problem hiding this comment.
Then in this case, it is better to always pass the header x-marqo-index-name: "" if no index name is provided, just so no request is missing the header.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behavior? (You can also link to an open issue here)
http requests only have api key in header (if present)
What is the new behavior (if this is a feature change)?
http requests send index name in header
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
no
Other information: