WIP Queries #379

ssanjay1 · 2024-06-06T17:36:24Z

WIP: Do not merge

ssanjay1 · 2024-06-18T17:38:44Z

packages/client/src/util/datamapping.ts

+ * limitations under the License.
+ */
+
+import type {


Need to delete this, ignore

ssanjay1 · 2024-06-26T21:33:27Z

packages/client/src/queries/applyQuery.ts

+      >;
+    }
+    case "object": {
+      const def = definitions.get(responseDataType.object);


Reading through this again, I'm not sure I actually need to do this since I should have captured the objectDef in OsdkTargetType

See my other comment about why I don't want to rely on the definitions in the outputted sdk at runtime.

ssanjay1 · 2024-06-26T21:34:18Z

packages/client/src/queries/applyQuery.ts

+      return responseValue as QueryReturnType<typeof responseDataType>;
+    }
+    case "twoDimensionalAggregation": {
+      const result: { key: any; value: any }[] = [];


These are any's right now, once I clean up the 2d/3d aggs type can clean this up

ssanjay1 · 2024-06-26T21:35:40Z

packages/client/src/queries/createQueryInvoker.ts

+  query: Q,
+): QuerySignatureFromDef<Q> {
+  return function(...args: any[]) {
+    return applyQuery(client, query, ...args);


Just kept consistent with what actions does for now, can switch over later

Yeah we can optimize later. I agree with getting it functionally correct for now.

ssanjay1 · 2024-06-26T21:38:25Z

packages/client/src/util/toDataValueQueries.ts

+  }
+
+  switch (desiredType.type) {
+    case "attachment": {


Now that we have the metadata of the query to help us convert to the right wire type, I'm not sure if we should throw if we don't get the right types. E.g. if the desiredType is an object, but what we got is not an OsdkBaseObject, should we throw here, or just pass through the value to the wire and rely on the backend to tell us something went wrong? I'm leaning on the side of us throwing here, but not sure

Behaviorally I think they will both result in an error thrown, it is just a question of how useful the error is. This might be a situation where in production we just pass through but in development we provide a lot of details. Once you get to production this type of logical error shouldnt exist.

packages/client/tsup.config.bundled_xnzkd6wirio.mjs

packages/generator/src/v1.1/generatePerQueryDataFiles.ts

packages/shared.test/src/handlers/loadObjectsEndpoints.ts

packages/client.api/src/mapping/DataValueMapping.ts

packages/client.api/src/queries/Queries.ts

ssanjay1 · 2024-06-27T13:30:03Z

packages/client/src/queries/applyQuery.ts

+      >;
+    }
+    case "object": {
+      const def = responseDataType.__OsdkTargetType


So if for some reason the objectDef wasn't capture in the type, I also just try manually loading the def from the ontologyProvider and then throw if both times I can't find it. Not sure if this is overkill though...

That capture of the type is just for typing, we dont actually codify it right? I think you always want to go to the provider

Changed to just use provider

packages/client/src/queries/applyQuery.ts

ericanderson · 2024-07-01T13:41:38Z

packages/api/src/ontology/QueryDefinition.ts

-  extends BaseQueryDataTypeDefinition<"object">
-{
+export interface ObjectQueryDataType<
+  K extends string,


Just curious, what does this string represent?

It just represents the object API name

packages/client.api/src/queries/Queries.ts

ericanderson · 2024-07-01T13:47:09Z

packages/client.api/src/queries/Queries.ts

+  [K in keyof T]: T[K] extends { nullable: true } ? never : K;
+}[keyof T];
+
+type NotOptionalParams<


Should this exclude optionals?

I think the PartialByNotStrict above should handle removing any optional param overlap for us right?

ericanderson · 2024-07-01T13:49:11Z

packages/client/src/queries/applyQuery.ts

+  ) {
+    const withoutMultiplicity = { ...responseDataType, multiplicity: false };
+    for (let i = 0; i < responseValue.length; i++) {
+      responseValue[i] = remapQueryResponse(


Its okay that this doesnt await?

Actually this function doesnt have any awaits? is responseValue[i] an any? This code is confusing me without looking at in in an IDE

Ah good catch, the tests for some of the nested types seemed to work fine without it, but addd the awaits in case. (Should also add a test for arrays I guess)

Yea responseValue[i] is any, because it's typed as DataValue which is just any

ericanderson · 2024-07-01T13:50:00Z

packages/client/src/queries/applyQuery.ts

+      >;
+    }
+    case "object": {
+      const def = responseDataType.__OsdkTargetType


That capture of the type is just for typing, we dont actually codify it right? I think you always want to go to the provider

ericanderson · 2024-07-01T13:57:30Z

packages/client/src/queries/queries.test.ts

+        key: "Q-AFO",
+        groups: [],
+      },
+    ]);


It would probably be good to check that it makes sense with unkown return types

ericanderson · 2024-07-01T13:58:35Z

packages/client/src/util/isOntologyObjectV2.ts

 import type { OntologyObjectV2 } from "@osdk/internal.foundry";

 export function isOntologyObjectV2(o: any): o is OntologyObjectV2 {
  return o && typeof o === "object" && typeof o.__apiName === "string"
    && o.__primaryKey != null;
 }
+
+export function isOsdkBaseObject(o: any): o is OsdkBase<any> {


Can we put this in another file? I dont think its related to the other function at all

ericanderson · 2024-07-01T14:00:36Z

packages/client/src/util/toDataValueQueries.ts

+  }
+
+  switch (desiredType.type) {
+    case "attachment": {


Behaviorally I think they will both result in an error thrown, it is just a question of how useful the error is. This might be a situation where in production we just pass through but in development we provide a lot of details. Once you get to production this type of logical error shouldnt exist.

packages/client/tsup.config.bundled_xnzkd6wirio.mjs

packages/generator/src/v1.1/generatePerQueryDataFiles.ts

ssanjay1 · 2024-07-03T17:30:09Z

examples-extra/basic/sdk/src/generatedNoCheck/ontology/queries/queryTakesAllParameterTypes.ts

+      type: 'objectSet',
+      objectSet: 'Todo',
+      nullable: false,
+      __OsdkTargetType: Todo,


@ericanderson is this not codifying the target type here? I may be misunderstanding your comment

Oh I see. Still wouldn't rely on it

To give more color, I want the option to remove the runtime cost of these and reduce them to just compile time types (for the most part). In order to do that, we need to not rely on this from bundling but instead to get the latest from the server before execution.

ericanderson

Only remaining thing is to be sure we arent using the runtime values for the definitions and loading from the server instead.

ericanderson · 2024-07-10T21:37:27Z

examples-extra/basic/sdk/src/generatedNoCheck/ontology/queries/queryTakesAllParameterTypes.ts

+      type: 'objectSet',
+      objectSet: 'Todo',
+      nullable: false,
+      __OsdkTargetType: Todo,


To give more color, I want the option to remove the runtime cost of these and reduce them to just compile time types (for the most part). In order to do that, we need to not rely on this from bundling but instead to get the latest from the server before execution.

ericanderson · 2024-07-10T21:38:51Z

packages/client/src/queries/applyQuery.ts

+      >;
+    }
+    case "object": {
+      const def = definitions.get(responseDataType.object);


See my other comment about why I don't want to rely on the definitions in the outputted sdk at runtime.

* Bump @typescript-eslint/parser from 7.9.0 to 7.16.0 (#445) * Bump @typescript-eslint/parser from 7.9.0 to 7.16.0 Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 7.9.0 to 7.16.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v7.16.0/packages/parser) --- updated-dependencies: - dependency-name: "@typescript-eslint/parser" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Fix example generation and group eslint-parser in the future --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Eric Anderson <[email protected]> * Bump pino-pretty from 11.0.0 to 11.2.1 (#471) Bumps [pino-pretty](https://github.com/pinojs/pino-pretty) from 11.0.0 to 11.2.1. - [Release notes](https://github.com/pinojs/pino-pretty/releases) - [Commits](pinojs/pino-pretty@v11.0.0...v11.2.1) --- updated-dependencies: - dependency-name: pino-pretty dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Version Packages (beta) (#463) * Add api-extractor, fix turbo, fix refactor (#415) * Add api-extractor, fix turbo, fix refactor * Add docs and allow CI to work * Fix action params issue with objects (#458) * fix * added changeset * Add more logging to legacy-client oauth (#479) * Change default VSCode formatter back to dprint (#482) * MRL enforces tilde not caret for internal deps (#464) * MRL enforces tilde not caret for internal deps * Cleanup * Interface inherited properties are now generated (#441) Fix mrl * WIP Queries (#379) * starting queries work * first pass, no tests * refactor * progress * Get objects working * object set support * add obejctset supoprt * remove console logs * remove newlines * remove unsused endpoints file * add 2d agg input support * 3d agg test * a little cleanup * fix empty index files * start addressing PR comments * codegen cleanup * only use provider for object defs * clean up bucket types * a little more codegen cleanup * nullable checks * add changeset * Beta generation initial work (#469) * Speed up build, fix dts generation for all, primitive --beta * Add empty changeset --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Eric Anderson <[email protected]> Co-authored-by: ssanjay1 <[email protected]>

ssanjay1 added 3 commits June 6, 2024 13:35

starting queries work

c5fd1e6

fix merge conflicts

64fe1f5

first pass, no tests

e1d1969

ssanjay1 commented Jun 18, 2024

View reviewed changes

ssanjay1 added 11 commits June 18, 2024 14:30

refactor

df61560

progress

3166a4e

Get objects working

4de0dc0

object set support

041f34d

fix merge conflict

ef866f4

add obejctset supoprt

7caaddb

remove console logs

3b0a0b0

remove newlines

173e4fc

remove unsused endpoints file

373ef2b

add 2d agg input support

8680c25

3d agg test

dd29ff3

ssanjay1 commented Jun 26, 2024

View reviewed changes

a little cleanup

0008a8b

ssanjay1 commented Jun 27, 2024

View reviewed changes

packages/shared.test/src/handlers/loadObjectsEndpoints.ts Show resolved Hide resolved

ssanjay1 commented Jun 27, 2024

View reviewed changes

packages/client.api/src/mapping/DataValueMapping.ts Show resolved Hide resolved

fix empty index files

5029221

ssanjay1 commented Jun 27, 2024

View reviewed changes

packages/client.api/src/queries/Queries.ts Show resolved Hide resolved

ssanjay1 commented Jun 27, 2024

View reviewed changes

packages/client/src/queries/applyQuery.ts Show resolved Hide resolved

ericanderson reviewed Jul 1, 2024

View reviewed changes

ssanjay1 commented Jul 3, 2024

View reviewed changes

ssanjay1 added 5 commits July 8, 2024 09:28

start addressing PR comments

0017f2b

codegen cleanup

ba7117e

only use provider for object defs

73c2988

clean up bucket types

e64f0a2

a little more codegen cleanup

5204063

ericanderson approved these changes Jul 10, 2024

View reviewed changes

ssanjay1 added 2 commits July 14, 2024 13:18

update branch

9ecc601

nullable checks

b5ccec3

ssanjay1 marked this pull request as ready for review July 14, 2024 18:02

add changeset

7acba23

ssanjay1 merged commit 3ec7c38 into main Jul 15, 2024
6 checks passed

ssanjay1 deleted the ssanjay/queries branch July 15, 2024 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Queries #379

WIP Queries #379

ssanjay1 commented Jun 6, 2024

ssanjay1 Jun 18, 2024

ssanjay1 Jun 26, 2024

ericanderson Jul 10, 2024

ssanjay1 Jun 26, 2024

ssanjay1 Jun 26, 2024

ericanderson Jul 1, 2024

ssanjay1 Jun 26, 2024 •

edited

Loading

ericanderson Jul 1, 2024

ssanjay1 Jun 27, 2024

ericanderson Jul 1, 2024

ssanjay1 Jul 10, 2024

ericanderson Jul 1, 2024

ssanjay1 Jul 3, 2024

ericanderson Jul 1, 2024

ssanjay1 Jul 3, 2024

ericanderson Jul 1, 2024

ericanderson Jul 1, 2024

ssanjay1 Jul 3, 2024

ssanjay1 Jul 3, 2024

ericanderson Jul 1, 2024

ericanderson Jul 1, 2024

ericanderson Jul 1, 2024

ericanderson Jul 1, 2024

ssanjay1 Jul 3, 2024

ericanderson Jul 4, 2024

ericanderson Jul 10, 2024

ericanderson left a comment

ericanderson Jul 10, 2024

ericanderson Jul 10, 2024

WIP Queries #379

WIP Queries #379

Conversation

ssanjay1 commented Jun 6, 2024

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

ssanjay1 Jun 26, 2024 • 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

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

ericanderson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssanjay1 Jun 26, 2024 •

edited

Loading