DRAFT: [Tables] API Review 1.0.0-preview.1 #1

mahdiva · 2020-07-28T18:23:24Z

No description provided.

joheredi · 2020-07-28T18:44:01Z

sdk/tables/azure-tables/review/tables.api.review.md

+
+// @public
+export type CreateEntityResponse = TableInsertEntityHeaders & {
+  [propertyName: string]: any;


WIP: CreateEntityResponse will use generic types

export type CreateEntityResponse<T> = TableInsertEntityHeaders & T & { // ... }

joheredi · 2020-07-28T18:45:21Z

sdk/tables/azure-tables/review/tables.api.review.md

+
+// @public
+export interface DeleteEntityOptions extends coreHttp.OperationOptions {
+  queryOptions?: GeneratedQueryOptions;


We'll need to rename GeneratedQueryOptions

joheredi · 2020-07-28T18:47:51Z

sdk/tables/azure-tables/review/tables.api.review.md

+}
+
+// @public
+export type Entity<T> = T & {


Other languages seem to be going with TableEntity. Is TableEntity what we want to use across languages?

Entity should also include etag and timestamp

joheredi · 2020-07-28T18:48:23Z

sdk/tables/azure-tables/review/tables.api.review.md

+};
+
+// @public
+export interface GeneratedQueryOptions {


We may want to rename GeneratedQueryOptions

joheredi · 2020-07-28T18:58:28Z

sdk/tables/azure-tables/review/tables.api.review.md

+export type ListEntitiesIterator<T extends object> = PagedAsyncIterableIterator<
+  T,
+  ListEntitiesPageResult<T>,
+  {}


WIP: We are working on refining the iterator. {} will be replaced by PageSettings or removed.

For context this is the interface of PageSettings

/** * @interface * An interface that tracks the settings for paged iteration */ export interface PageSettings { /** * @member {string} [continuationToken] The token that keeps track of where to continue the iterator */ continuationToken?: string; /** * @member {number} [pageSize] The size of the page during paged iteration */ maxPageSize?: number; }

joheredi · 2020-07-28T19:25:38Z

sdk/tables/azure-tables/review/tables.api.review.md

+  date?: Date;
+  requestId?: string;
+  version?: string;
+  xMsContinuationNextTableName?: string;


xMsContinuationNextTableName should probably be renamed to nextTableName

joheredi · 2020-07-28T19:26:19Z

sdk/tables/azure-tables/review/tables.api.review.md

+
+// @public
+export class TableServiceClient {
+  constructor(url: string, options?: TableServiceClientOptions);


WIP: constructor override which accepts SharedKeyCredential

joheredi · 2020-07-28T19:28:51Z

sdk/tables/azure-tables/review/tables.api.review.md

+    query?: QueryOptions,
+    options?: ListEntitiesOptions
+  ): Promise<ListEntitiesResponse<T>>;
+  listTables(query?: QueryOptions, options?: ListTablesOptions): Promise<ListTablesResponse>;


WIP: listTables will support PagedAsyncIterableIterator

joheredi · 2020-07-28T19:30:17Z

sdk/tables/azure-tables/review/tables.api.review.md

+  queryOptions?: GeneratedQueryOptions;
+  requestId?: string;
+  tableEntityProperties?: {
+    [propertyName: string]: any;


If possible we should override uses of [propertyName: string]: any] with generic types

joheredi · 2020-07-28T19:31:56Z

sdk/tables/azure-tables/review/tables.api.review.md

+    tableName: string,
+    entity: Entity<T>,
+    mode?: UpdateMode,
+    eTag?: string,


If we are already passing the entity, do we need to also pass the eTag? Or can we just extract it from the entity we were passed?

Do we want to default eTag to "*" or do we make it required and don't assume "*" on undefined?

"*" will force unconditional update

From the arch board review, we'd like to have etag as optional and default to "*"

richardpark-msft · 2020-07-28T19:57:19Z

sdk/tables/azure-tables/review/tables.api.review.md

+// @public
+export interface GeneratedQueryOptions {
+  filter?: string;
+  format?: OdataMetadataFormat;


So looking at this the actual values the user would pass here are things like this:
"application/json;odata=nometadata"

Is the user expected to modify these strings at all (ie, pass something other applictio/json, like octet-stream, etc..)?

If not is it worth simplifying them? Perhaps:
"none" | "minimal" | "full"

and then translating this in your convenience layer?

richardpark-msft · 2020-07-28T19:58:24Z

sdk/tables/azure-tables/review/tables.api.review.md

+}
+
+// @public
+export function odata(strings: TemplateStringsArray, ...values: unknown[]): string;


It seems somewhat rare to have an exported function without some associated class. How would the user use this - just curious how it ties in.

This helper function will be used as a tagged template literal for escaping any single quotes inside a filter parameter when querying.

That is pretty neat!

richardpark-msft · 2020-07-28T19:59:00Z

sdk/tables/azure-tables/review/tables.api.review.md

+  | "application/json;odata=fullmetadata";
+
+// @public
+export interface QueryOptions extends Omit<GeneratedQueryOptions, "select"> {


I love Omit<> but be aware that it does ugly thing in our doc tool currently.

richardpark-msft · 2020-07-28T20:03:49Z

sdk/tables/azure-tables/review/tables.api.review.md

+// @public
+export interface TableSetAccessPolicyOptionalParams extends coreHttp.OperationOptions {
+  requestId?: string;
+  tableAcl?: SignedIdentifier[];


Not familiar with tables but is this always one way (ie only acts as an allowed list, never acts as a deny list?).

richardpark-msft · 2020-07-28T20:05:37Z

sdk/tables/azure-tables/review/tables.api.review.md

+export interface TableSetAccessPolicyOptionalParams extends coreHttp.OperationOptions {
+  requestId?: string;
+  tableAcl?: SignedIdentifier[];
+  timeout?: number;


I believe we have a rule about this - it has to be timeoutInMs (ie, mention the unit and add 'In').

Also, is this timeout separate from the normal corehttp timeouts? Does it function independently of our normal retries, etc.?

richardpark-msft · 2020-07-28T20:06:51Z

sdk/tables/azure-tables/review/tables.api.review.md

+
+// @public
+export interface TableSetAccessPolicyOptionalParams extends coreHttp.OperationOptions {
+  requestId?: string;


I'm curious what this is - I feel like generically we have fields with 'request' in the name all over the place (ie, for http, etc..). Is this something specific o tables (and if so - could it have a more Tables specific name?)

richardpark-msft · 2020-07-28T20:08:23Z

sdk/tables/azure-tables/review/tables.api.review.md

+};
+
+// @public
+export enum UpdateMode {


In Service Bus we've been eschewing enums in favor of just using string unions - no sure if that's an option you'd like to try here or if the 0/1 is better for clients.

So:

export type UpdateMode = "merge" | "replace";

richardpark-msft · 2020-07-28T20:09:59Z

sdk/tables/azure-tables/review/tables.api.review.md

+    tableName: string,
+    partitionKey: string,
+    rowKey: string,
+    ifMatch: string,


Is ifMatch really not optional? Or is this not the etag ifMatch?

richardpark-msft · 2020-07-28T20:12:30Z

sdk/tables/azure-tables/review/tables.api.review.md

+}
+
+// @public
+export class EdmInt64 {


This is interesting! We use a long package in Service Bus and I seem to recall Brian pushing hard to see if we could just use number, etc...

Do you know if we have an "approved" way of representing longer number at this point?

@richardpark-msft, your bring up a good point. We're currently having this conversation with @bterlson, @xirzec and @ellismg

It seems like we'll land on not having these classes and instead representing these "special" types with a JSON object and give the users freedom to choose the big integer library of they liking.

{ id: {value: "60c1ebb2-875e-4988-8610-efe28a600c08", type: "Guid"}, viewCount: {value: "55999333", type: "Int64"}, name: "Some entity", price: 123.50, isActive: true, inStockUntil: new Date("11/05/2020"); }

So for example if the user has BigInt they can do:

c = obj.viewCount.type === 'Int64' ? BigInt(obj.viewCount.value) : obj.viewCount

Note that these Edm objects will only be needed when users need to explicitly set Edm.Guid and Edm.Int64 in the Service all the other supported types will be handled by the SDK and represented as native types.

Are you taking community feedback on the EdmGuid and EdmInt64 class types? I've been watching this closely and really like the class representation versus the object representation. The object representation could also be used, as the classes could have the proposed properties in the object which would allow the same check of the "type" property.

richardpark-msft · 2020-07-28T20:14:19Z

sdk/tables/azure-tables/samples/typescript/src/TableServiceClientSamples.ts

+}
+
+async function listTables() {
+  const tables = await serviceClient.listTables({


(sometimes it's interesting to "document" the defaults in the sample.

So for instance, is there a default sort order, that kind of thing and just place the field with it's default value, indicating it's the default. I think this is just for your review, so no big deal, but something to consider.

joheredi · 2020-07-29T22:57:38Z

sdk/tables/azure-tables/samples/typescript/src/TableServiceClientSamples.ts

+}
+
+async function deleteEntity() {
+  await serviceClient.deleteEntity("Samples", "Part1", "Row4", "*");


Jeffrey Ritcher's comment: Do we want to hide entity operations from ServiceClient and only expose from the TableClient?

meanjeekim mentioned this pull request Jul 28, 2020

Board Review: Tables Azure/azure-sdk#1589

Closed

joheredi reviewed Jul 28, 2020

View reviewed changes

richardpark-msft reviewed Jul 28, 2020

View reviewed changes

Tables API Extractor Review

136a19c

mahdiva force-pushed the tables-api-review branch from e9a8368 to 136a19c Compare July 29, 2020 21:05

joheredi reviewed Jul 29, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT: [Tables] API Review 1.0.0-preview.1 #1

DRAFT: [Tables] API Review 1.0.0-preview.1 #1

mahdiva commented Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 29, 2020 •

edited

Loading

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 28, 2020

joheredi Jul 29, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020 •

edited

Loading

mahdiva Jul 29, 2020

richardpark-msft Aug 5, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020

richardpark-msft Jul 28, 2020

joheredi Jul 28, 2020 •

edited

Loading

ahuggins-nhs Aug 4, 2020

richardpark-msft Jul 28, 2020

joheredi Jul 29, 2020

DRAFT: [Tables] API Review 1.0.0-preview.1 #1

Are you sure you want to change the base?

DRAFT: [Tables] API Review 1.0.0-preview.1 #1

Conversation

mahdiva commented Jul 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joheredi Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richardpark-msft Jul 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joheredi Jul 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joheredi Jul 29, 2020 •

edited

Loading

richardpark-msft Jul 28, 2020 •

edited

Loading

joheredi Jul 28, 2020 •

edited

Loading