Upgrade core dependencies to 5.0 #225

pmconne · 2024-12-24T14:50:00Z

iTwin/itwinjs-core#7512 makes the internal APIs IModelDb.nativeDb and IModelHost.platform inaccessible outside of the core-backend package.

…ngeset.

# Conflicts: # pnpm-lock.yaml

pmconne · 2025-01-03T13:07:19Z

@iTwin/transformation-service @iTwin/itwinjs-core-transformer are the performance tests in rawInserts.ts actually run and the results evaluated, and if so is it necessary for them to apply changesets like they currently do? I'd prefer not to introduce a public API for applying changesets to a standalone iModel as that concept is nonsensical.

packages/performance-tests build$ tsc 1>&2
│ test/rawInserts.ts(87,17): error TS2339: Property 'nativeDb' does not exist on type 'StandaloneDb'.
│ test/rawInserts.ts(190,28): error TS2339: Property 'nativeDb' does not exist on type 'IModelDb'.
│ test/rawInserts.ts(199,10): error TS2339: Property 'nativeDb' does not exist on type 'IModelDb'.
└─ Failed in 2.8s at /home/paul/c/j2/s/imodel-transformer/packages/performance-tests

nick4598 · 2025-01-06T19:42:54Z

@iTwin/transformation-service @iTwin/itwinjs-core-transformer are the performance tests in rawInserts.ts actually run and the results evaluated, and if so is it necessary for them to apply changesets like they currently do? I'd prefer not to introduce a public API for applying changesets to a standalone iModel as that concept is nonsensical.
packages/performance-tests build$ tsc 1>&2
│ test/rawInserts.ts(87,17): error TS2339: Property 'nativeDb' does not exist on type 'StandaloneDb'.
│ test/rawInserts.ts(190,28): error TS2339: Property 'nativeDb' does not exist on type 'IModelDb'.
│ test/rawInserts.ts(199,10): error TS2339: Property 'nativeDb' does not exist on type 'IModelDb'.
└─ Failed in 2.8s at /home/paul/c/j2/s/imodel-transformer/packages/performance-tests

These numbers are collected and visualized in a powerBi report. As for how often they're looked at, I couldn't say. How else can we apply changesets? The only way I could think would be to create a new iModel on the hub each time the test is run and clean it up afterwards? Is that what you were picturing as well?

pmconne · 2025-01-06T20:10:58Z

How else can we apply changesets?

Why is imodel-transformer measuring time required to apply changesets? That's totally outside its purview.

nick4598 · 2025-01-06T20:20:36Z

Why is imodel-transformer measuring time required to apply changesets? That's totally outside its purview.

I'm sort of guessing here, but I suppose its to show a comparison between 3 things (in order of fastest to slowest):
"populate by applying changeset"
"populate by insert"
"populate by transform"

I think "applying changeset" and "insert" serve to show how much time is added by using the transformer. Perhaps "applying changeset" is unnecessary and we could just use the "insert" to achieve that. I don't know of another reason for the applying changesets test.

pmconne · 2025-01-06T20:21:23Z

I think "applying changeset" and "insert" serve to show how much time is added by using the transformer.

That was my understanding too.

Perhaps "applying changeset" is unnecessary and we could just use the "insert" to achieve that.

That'd be my preference.

nick4598 · 2025-01-06T20:44:37Z

Removed it

…nsformer into pmc/no-nativedb

pmconne · 2025-01-07T12:46:27Z

Unclear what's wrong with extract-api.js. It spit out csv files but no .api.md files. Seems like it must be finding an older version of node or tsc, but if so I can't tell from where.

$> pnpm --loglevel debug run extract-api

> @itwin/imodel-transformer@1.0.0 extract-api /home/paul/c/j2/s/imodel-transformer/packages/transformer
> betools extract-api --entry=imodel-transformer --apiReportFolder=../../common/api --apiReportTempFolder=../../common/api/temp --apiSummaryFolder=../../common/api/summary

Running command:
node /home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+build-tools@5.0.0-dev.34_@types+node@18.19.55/node_modules/@itwin/build-tools/scripts/extract-api.js --entry imodel-transformer --apiReportFolder ../../common/api --apiReportTempFolder ../../common/api/temp --apiSummaryFolder ../../common/api/summary

api-extractor 7.47.12  - https://api-extractor.com/


ERROR: Error parsing tsconfig.json content: Argument for '--target' option must be: 'es5', 'es6', 'es2015', 'es2016', 'es2017', 'es2018', 'es2019', 'es2020', 'es2021', 'es2022', 'esnext'.

$> pnpm --version
8.15.7
$> whereis pnpm
pnpm: /home/paul/.nvm/versions/node/v20.18.0/bin/pnpm
$> node -v
v20.18.0
$> whereis tsc
tsc: /home/paul/.nvm/versions/node/v20.18.0/bin/tsc
$> tsc -v
Version 5.6.2

pmconne · 2025-01-07T13:59:35Z

Current test failures (2)

  1) IModelTransformer
       transforms code values with non standard space characters:

      AssertionError: expected 'SpatialCategory ' to equal 'SpatialCategory'
      + expected - actual

      -SpatialCategory
      +SpatialCategory

      at /home/paul/c/j2/s/imodel-transformer/packages/transformer/src/test/standalone/IModelTransformer.test.ts:4125:34
      at SnapshotDb.withSqliteStatement (/home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+core-backend@5.0.0-dev.34_@itwin+core-bentley@5.0.0-dev.34_@itwin+core-common@5.0.0-de_fc57ame2wv7ewisulld4qqxfwm/node_modules/@itwin/core-backend/src/IModelDb.ts:642:22)
      at getCodeValRawSqlite (src/test/standalone/IModelTransformer.test.ts:4119:10)
      at Context.<anonymous> (src/test/standalone/IModelTransformer.test.ts:4236:7)

  2) IModelTransformerHub
       should delete definition elements and models when processing changes:
     Missing Id: error deleting model [missing id] id 0x20000000006
      at /home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+core-backend@5.0.0-dev.34_@itwin+core-bentley@5.0.0-dev.34_@itwin+core-common@5.0.0-de_fc57ame2wv7ewisulld4qqxfwm/node_modules/@itwin/core-backend/src/IModelDb.ts:1771:17
      at Set.forEach (<anonymous>)
      at Models.deleteModel (/home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+core-backend@5.0.0-dev.34_@itwin+core-bentley@5.0.0-dev.34_@itwin+core-common@5.0.0-de_fc57ame2wv7ewisulld4qqxfwm/node_modules/@itwin/core-backend/src/IModelDb.ts:1767:25)
      at SpecialElements.deleteSpecialElements (/home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+core-backend@5.0.0-dev.34_@itwin+core-bentley@5.0.0-dev.34_@itwin+core-common@5.0.0-de_fc57ame2wv7ewisulld4qqxfwm/node_modules/@itwin/core-backend/src/ElementTreeWalker.ts:283:23)
      at ElementCascadingDeleter.deleteSpecialElements (/home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+core-backend@5.0.0-dev.34_@itwin+core-bentley@5.0.0-dev.34_@itwin+core-common@5.0.0-de_fc57ame2wv7ewisulld4qqxfwm/node_modules/@itwin/core-backend/src/ElementTreeWalker.ts:343:19)
      at deleteElementTreeCascade (src/ElementCascadingDeleter.ts:25:7)
      at IModelImporter.onDeleteElement (src/IModelImporter.ts:399:29)
      at IModelImporter.deleteElement (src/IModelImporter.ts:416:10)
      at IModelTransformer.onDeleteElement (src/IModelTransformer.ts:1938:21)
      at IModelExporter.exportChanges (src/IModelExporter.ts:480:24)
      at IModelTransformer.processChanges (src/IModelTransformer.ts:3237:5)
      at runTimeline (src/test/TestUtils/TimelineTestUtil.ts:525:11)
      at Context.<anonymous> (src/test/standalone/IModelTransformerHub.test.ts:4650:42)

nick4598 · 2025-01-07T14:18:09Z

Current test failures (2)

Im guessing those are both caused by the semver checks we do in transformer and dev versions tripping it up.

Would you like this PR merged soon? My concern is that multiple teams seems to be using this package (not sure to what extent), and we should probably bump the major version of transformer if we're increasing our peer to be minimum 5.0.

pmconne · 2025-01-07T14:30:28Z

Im guessing those are both caused by the semver checks we do in transformer and dev versions tripping it up.

Could you explain? The failures don't seem semver-related to me. The first one looks like a change in behavior (possible regression) in the core packages.

Would you like this PR merged soon? My concern is that multiple teams seems to be using this package (not sure to what extent), and we should probably bump the major version of transformer if we're increasing our peer to be minimum 5.0.

/s/probably/definitely, that's the point of this PR - to upgrade imodel-transformer to use iTwin.js 5.0 as part of its own major version update. Are other changes planned for this package in either the 4.x or 5.x timeline?

nick4598 · 2025-01-07T16:29:45Z

Could you explain? The failures don't seem semver-related to me. The first one looks like a change in behavior (possible regression) in the core packages.

https://github.com/iTwin/imodel-transformer/blob/main/packages/transformer/src/test/standalone/IModelTransformer.test.ts#L4237-L4250

The first failing test checks 'inITwinJSVersionWithExactCodeFeature', this check may be the problem. I could be wrong.

The second failing test goes through this code path I believe: https://github.com/iTwin/imodel-transformer/blob/main/packages/transformer/src/IModelTransformer.ts#L908-L920

Luckily we could probably remove these semver checks with the update to 5.0.

/s/probably/definitely, that's the point of this PR - to upgrade imodel-transformer to use iTwin.js 5.0 as part of its own major version update. Are other changes planned for this package in either the 4.x or 5.x timeline?

There is a currently active PR #172 that i have been working on alongside Julija from transformation-services team. I'd prefer to get that in before updating to 5.x in master. Any opinions on potentially having this PR target a v5 branch as opposed to main? I suppose we could also consider creating a 4.x branch instead and having this go to master.

pmconne · 2025-01-07T17:28:56Z

Any opinions on potentially having this PR target a v5 branch as opposed to main? I suppose we could also consider creating a 4.x branch instead and having this go to master.

I have no opinions, whatever works for this repo.

nick4598 · 2025-01-08T14:13:46Z

Unclear what's wrong with extract-api.js. It spit out csv files but no .api.md files. Seems like it must be finding an older version of node or tsc, but if so I can't tell from where.

$> pnpm --loglevel debug run extract-api

> @itwin/imodel-transformer@1.0.0 extract-api /home/paul/c/j2/s/imodel-transformer/packages/transformer
> betools extract-api --entry=imodel-transformer --apiReportFolder=../../common/api --apiReportTempFolder=../../common/api/temp --apiSummaryFolder=../../common/api/summary

Running command:
node /home/paul/c/j2/s/imodel-transformer/node_modules/.pnpm/@itwin+build-tools@5.0.0-dev.34_@types+node@18.19.55/node_modules/@itwin/build-tools/scripts/extract-api.js --entry imodel-transformer --apiReportFolder ../../common/api --apiReportTempFolder ../../common/api/temp --apiSummaryFolder ../../common/api/summary

api-extractor 7.47.12  - https://api-extractor.com/


ERROR: Error parsing tsconfig.json content: Argument for '--target' option must be: 'es5', 'es6', 'es2015', 'es2016', 'es2017', 'es2018', 'es2019', 'es2020', 'es2021', 'es2022', 'esnext'.

$> pnpm --version
8.15.7
$> whereis pnpm
pnpm: /home/paul/.nvm/versions/node/v20.18.0/bin/pnpm
$> node -v
v20.18.0
$> whereis tsc
tsc: /home/paul/.nvm/versions/node/v20.18.0/bin/tsc
$> tsc -v
Version 5.6.2

@DanRod1999 can you take a look at what may be going on when you get a chance? I was reproducing locally, maybe worth mentioning that changing ONLY @itwin/build-tools to 4.10.3 from 5.0.0-dev.34 seemed to resolve the issue.

…oft extractor api

package.json

…n microsoft extractor api" This reverts commit b1b612e.

pmconne · 2025-01-27T11:11:59Z

@nick4598 @DanRod1999 @ColinKerr or @MichaelSwigerAtBentley please review and merge once satisfied.

DanRod1999 · 2025-01-27T14:56:19Z

packages/transformer/src/test/standalone/IModelTransformerHub.test.ts

@@ -3536,7 +3536,7 @@ describe("IModelTransformerHub", () => {
      rootSubject: { name: masterIModelName },
    });
    // eslint-disable-next-line deprecation/deprecation
-    masterSeedDb.nativeDb.setITwinId(iTwinId); // workaround for "ContextId was not properly setup in the checkpoint" issue
+    // masterSeedDb.nativeDb.setITwinId(iTwinId); // workaround for "ContextId was not properly setup in the checkpoint" issue


is this workaround no longer needed, and are we just keeping these here for future reference?

I frankly have no idea, insufficient information provided in comment.
CI checks are not running on this PR any more because target branch was changed. Maybe some integration tests would fail?

pmconne added 8 commits December 24, 2024 09:32

rm unused flushTxns

403c057

update core pkgs, need to update clients etc

c54c3a3

upgrade other @iTwin packages to latest; still some unmet peer deps

23f734e

upgrade TS

5377bab

wip clean up compilation errors.

175e5c6

fix everything except performance tests that want to apply single cha…

a73834f

…ngeset.

Merge branch 'main' into pmc/no-nativedb

dea39ab

# Conflicts: # pnpm-lock.yaml

update lockfile

a6948be

pmconne marked this pull request as ready for review January 3, 2025 13:04

pmconne requested review from a team as code owners January 3, 2025 13:04

pmconne mentioned this pull request Jan 6, 2025

When releasing a version of imodel-transformer that depends on itwinjs 5.0 #227

Open

remove apply changeset test

d60fd14

pmconne added 6 commits January 7, 2025 05:54

fix up remaining callers

4356714

Merge branch 'pmc/no-nativedb' of https://github.com/iTwin/imodel-tra…

ba75299

…nsformer into pmc/no-nativedb

Change files

a12ae45

extract-api??

eade4dc

pnpm update

7fdfa60

come on what's your problem

bef74dc

rm font check

16e3468

pmconne added 2 commits January 7, 2025 12:36

fix 1 semver test

445245a

rm another semver check

a1a1890

nick4598 changed the base branch from main to itwinjs-v5 January 8, 2025 13:53

update package.json, set override on typsescript due to dep in micros…

b1b612e

…oft extractor api

GytisCepk reviewed Jan 15, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

pmconne added 6 commits January 27, 2025 06:01

Revert "update package.json, set override on typsescript due to dep i…

0d4a556

…n microsoft extractor api" This reverts commit b1b612e.

dev.49

eeea61e

breaking changes to map iterators.

79cf7c1

plan projection settings

dfa8339

fix import

89d9924

extract-api

d2dd977

DanRod1999 reviewed Jan 27, 2025

View reviewed changes

DanRod1999 approved these changes Jan 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade core dependencies to 5.0 #225

Upgrade core dependencies to 5.0 #225

pmconne commented Dec 24, 2024 •

edited

Loading

pmconne commented Jan 3, 2025 •

edited

Loading

nick4598 commented Jan 6, 2025

pmconne commented Jan 6, 2025

nick4598 commented Jan 6, 2025

pmconne commented Jan 6, 2025

nick4598 commented Jan 6, 2025

pmconne commented Jan 7, 2025

pmconne commented Jan 7, 2025 •

edited

Loading

nick4598 commented Jan 7, 2025 •

edited

Loading

pmconne commented Jan 7, 2025

nick4598 commented Jan 7, 2025 •

edited

Loading

pmconne commented Jan 7, 2025

nick4598 commented Jan 8, 2025

pmconne commented Jan 27, 2025

DanRod1999 Jan 27, 2025

pmconne Jan 27, 2025

Upgrade core dependencies to 5.0 #225

Are you sure you want to change the base?

Upgrade core dependencies to 5.0 #225

Conversation

pmconne commented Dec 24, 2024 • edited Loading

pmconne commented Jan 3, 2025 • edited Loading

nick4598 commented Jan 6, 2025

pmconne commented Jan 6, 2025

nick4598 commented Jan 6, 2025

pmconne commented Jan 6, 2025

nick4598 commented Jan 6, 2025

pmconne commented Jan 7, 2025

pmconne commented Jan 7, 2025 • edited Loading

nick4598 commented Jan 7, 2025 • edited Loading

pmconne commented Jan 7, 2025

nick4598 commented Jan 7, 2025 • edited Loading

pmconne commented Jan 7, 2025

nick4598 commented Jan 8, 2025

pmconne commented Jan 27, 2025

DanRod1999 Jan 27, 2025

Choose a reason for hiding this comment

pmconne Jan 27, 2025

Choose a reason for hiding this comment

pmconne commented Dec 24, 2024 •

edited

Loading

pmconne commented Jan 3, 2025 •

edited

Loading

pmconne commented Jan 7, 2025 •

edited

Loading

nick4598 commented Jan 7, 2025 •

edited

Loading

nick4598 commented Jan 7, 2025 •

edited

Loading