Add config option to override the payer namespace for REST requests. #5105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
teo-tsirpanis
changed the title
eric-hughes-tiledb
previously requested changes
Jun 19, 2024
test/src/unit-curl.cc
Outdated
Comment on lines
141
to
142
| // We copy because the [] operator is not const-qualified. | ||
| auto extra_headers = resources.rest_client()->extra_headers(); |
There was a problem hiding this comment.
vector::at is const-qualified.
Similar to S3 custom header support this PR adds support for customer header in REST request. This allows users to set header via a config parameter instead of the current `tildb_context_set_tag`. The context set tag does not work in all cases because its not always possible to use the same context, i.e SOMA and VCF python APIs can't pass a TileDB-Py ctx to their respective C++ code.
Co-authored-by: eric-hughes-tiledb <[email protected]>
This is a new config that allows users to set the namespace to charge for requests when it cannot be automatically determined or when the user needs to set a specific override.
Co-authored-by: eric-hughes-tiledb <[email protected]>
KiterLuc
force-pushed
the
payer-namespace
branch
from
86077fe to
2f929ac
Compare
KiterLuc
approved these changes
Jun 19, 2024
ihnorton
approved these changes
Jun 19, 2024
There was a problem hiding this comment.
Tested and confirmed -- succeeds with a non-default namespace I have access to, and fails with an invalid namespace (and I see the X-payer header in logs). Thanks for the quick turnaround.
In [1]: import tiledb, numpy as np, tiledb.cloud
In [2]: tiledb.default_ctx(tiledb.cloud.Ctx().config().dict() | {"rest.payer_namespace": "foobar"})
Out[2]: tiledb.Ctx() [see Ctx.config() for configuration]
In [3]: a = tiledb.open("tiledb://ihnorton/<array>")
TileDBError: [TileDB::REST] Error: Error submitting query to REST; server returned no data. Curl error: Error in libcurl POST operation: libcurl error message 'CURLE_OK'; HTTP code 401; server response data '{"code":147,"message":"You are not allowed to charge queries to foobar","request_id":"965a4a52-c66a-467d-8372-d5556d85d082"}'.
KiterLuc
reviewed
Jun 19, 2024
KiterLuc
added a commit
that referenced
this pull request
Jun 19, 2024
…5105) [SC-49142](https://app.shortcut.com/tiledb-inc/story/49142/add-support-for-rest-x-payer-via-config) This PR introduces a new config option that allows users to set the namespace to charge for REST requests when it cannot be automatically determined or when the user needs to set a specific override. Big thanks to @Shelnutt2 for implementing this. I added a test and made some small changes on top of it. --- TYPE: CONFIG DESC: Add `rest.payer_namespace` config option to set the namespace to be charged for REST requests. --------- Co-authored-by: Seth Shelnutt <[email protected]> Co-authored-by: KiterLuc <[email protected]> Co-authored-by: eric-hughes-tiledb <[email protected]> Co-authored-by: Luc Rancourt <[email protected]>
KiterLuc
added a commit
that referenced
this pull request
Jun 19, 2024
…5105) [SC-49142](https://app.shortcut.com/tiledb-inc/story/49142/add-support-for-rest-x-payer-via-config) This PR introduces a new config option that allows users to set the namespace to charge for REST requests when it cannot be automatically determined or when the user needs to set a specific override. Big thanks to @Shelnutt2 for implementing this. I added a test and made some small changes on top of it. --- TYPE: CONFIG DESC: Add `rest.payer_namespace` config option to set the namespace to be charged for REST requests. --------- Co-authored-by: Seth Shelnutt <[email protected]> Co-authored-by: KiterLuc <[email protected]> Co-authored-by: eric-hughes-tiledb <[email protected]> Co-authored-by: Luc Rancourt <[email protected]>
KiterLuc
added a commit
that referenced
this pull request
Jun 19, 2024
…pace for REST requests. (#5105) (#5127) Backport bdfc19c from #5105. --- TYPE: CONFIG DESC: Add `rest.payer_namespace` config option to set the namespace to be charged for REST requests. Co-authored-by: Theodore Tsirpanis <[email protected]> Co-authored-by: Seth Shelnutt <[email protected]> Co-authored-by: eric-hughes-tiledb <[email protected]>
ihnorton
pushed a commit
that referenced
this pull request
Jun 20, 2024
…pace for REST requests. (#5105) (#5126) Backport bdfc19c from #5105. --- TYPE: CONFIG DESC: Add `rest.payer_namespace` config option to set the namespace to be charged for REST requests. Co-authored-by: Theodore Tsirpanis <[email protected]> Co-authored-by: Seth Shelnutt <[email protected]> Co-authored-by: eric-hughes-tiledb <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SC-49142
This PR introduces a new config option that allows users to set the namespace to charge for REST requests when it cannot be automatically determined or when the user needs to set a specific override.
Big thanks to @Shelnutt2 for implementing this. I added a test and made some small changes on top of it.
TYPE: CONFIG
DESC: Add
rest.payer_namespaceconfig option to set the namespace to be charged for REST requests.