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

QueryTablesIncludeFlags doesn't seem to be used anywhere within the QueryTables operation #6155

Open
ahsonkhan opened this issue Oct 30, 2024 · 3 comments · Fixed by #6156
Open
Assignees
Labels
blocking-release Blocks release bug This issue requires a change to an existing behavior in the product in order to be resolved. Tables
Milestone

Comments

@ahsonkhan
Copy link
Member

This option doesn't seem to be used by the SDK, in the implementation detail, but is publicly exposed:
#6154

It's probably worth going back to a source of truth and investigating. What does the REST API specs or service docs say here?

Maybe it was carried over from storage, where "include" is added as query parameter. Should the same be done here?

if (options.Include.HasValue()
&& !ListBlobContainersIncludeFlagsToString(options.Include.Value()).empty())
{
request.GetUrl().AppendQueryParameter(
"include",
_internal::UrlEncodeQueryParameter(
ListBlobContainersIncludeFlagsToString(options.Include.Value())));
}

GoLang doesn't seem to have this option, though I am not familiar with the service or specifics of Go's implementation:
https://github.com/Azure/azure-sdk-for-go/blob/0387c89ceebae256c9ea717533c739b7a875d996/sdk/data/aztables/options.go#L115-L145

@ahsonkhan ahsonkhan added bug This issue requires a change to an existing behavior in the product in order to be resolved. Tables labels Oct 30, 2024
@ahsonkhan ahsonkhan added this to the 2024-11 milestone Oct 30, 2024
@ahsonkhan
Copy link
Member Author

Marking as blocking release if we need to remove it, since this is public surface area, and can't be fixed after.

cc @RickWinter, @LarryOsterman

@ahsonkhan ahsonkhan added the blocking-release Blocks release label Oct 30, 2024
@ahsonkhan
Copy link
Member Author

The REST docs for query tables don't mention any use of the include query param.
https://learn.microsoft.com/en-us/rest/api/storageservices/query-tables

But, they do mention setting the Accept request header to one of a few sets of Metadata values (one of application/json;odata=nometadata, application/json;odata=minimalmetadata, application/json;odata=fullmetadata:
https://learn.microsoft.com/en-us/rest/api/storageservices/query-tables#request-headers

We seem to hardcode it to minimalmetadata always (and only in the PrepXEntity operations).

returnValue += "Accept: application/json;odata=minimalmetadata\n";

GoLang exposes this via options:
https://github.com/Azure/azure-sdk-for-go/blob/0387c89ceebae256c9ea717533c739b7a875d996/sdk/data/aztables/options.go#L129-L131
https://github.com/Azure/azure-sdk-for-go/blob/0387c89ceebae256c9ea717533c739b7a875d996/sdk/data/aztables/constants.go#L59-L67

cc @jhendrixMSFT

@ahsonkhan
Copy link
Member Author

The other languages expose the Metadata option which let's users modify the accept request header when calling QueryTables. The REST API doc linked above mentions it.

Re-opening to consider what we want to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-release Blocks release bug This issue requires a change to an existing behavior in the product in order to be resolved. Tables
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants