Add the latest version of oracledb to api/package.json #19067

stevefan1999-personal · 2023-07-05T15:22:27Z

~~Judging from https://github.com/directus/directus/actions/runs/5424640097/jobs/9864290720, there is only one unit test not being able to pass. Looks like we can just add it to the mix now.~~ This is not correct and it seems like the version is still pinned to 5.

I need this because trying to install NPM package from Docker image externally is very painful, here's a workaround I tried:

ARG VERSION

FROM directus/directus:${VERSION}

USER root
RUN chmod 777 -R /usr/local/bin/
WORKDIR /directus
RUN corepack enable && corepack prepare

RUN pnpm install oracledb --save
USER node

Clearly it is not elegant since it will bypass the pnpm pack:

directus/Dockerfile

Lines 14 to 28 in 816f7dc

    
           COPY pnpm-lock.yaml . 
        
           RUN pnpm fetch 
        
           COPY . . 
        
           RUN pnpm install --recursive --offline --frozen-lockfile 
        
           RUN : \ 
        
           	&& npm_config_workspace_concurrency=1 pnpm run build \ 
        
           	&& pnpm --filter directus deploy --prod dist \ 
        
           	&& cd dist \ 
        
           	&& pnpm pack \ 
        
           	&& tar -zxvf *.tgz package/package.json \ 
        
           	&& mv package/package.json package.json \ 
        
           	&& rm -r *.tgz package \ 
        
           	&& mkdir -p database extensions uploads \ 
        
           	;

Let alone the dependencies should be frozen and never to be updated again.

changeset-bot · 2023-07-05T15:22:31Z

🦋 Changeset detected

Latest commit: ea36f5b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@directus/api	Minor
directus	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

stevefan1999-personal · 2023-07-05T15:43:26Z

I can't regenerate pnpm lockfile, please do this for me

@stevefan1999-personal ➜ /workspaces/directus (patch-2) $ pnpm install --recursive --offline 
Scope: all 34 workspace projects
/workspaces/directus/docs:
 ERR_PNPM_NO_OFFLINE_META  Failed to resolve @paescuj/eslint-plugin-prettier@4.3.0-1 in package mirror /home/codespace/.cache/pnpm/metadata/registry.npmjs.org/@paescuj/eslint-plugin-prettier.json

Update: nevermind

stevefan1999-personal · 2023-07-05T15:59:44Z

Seeks like we are still using oracledb 5.x.x, hmm so the test result is not representative...

rijkvanzanten · 2023-07-05T16:11:07Z

The node Oracledb package can't be installed under ARM platforms (eg any modern mac), so this is not something we can add to the dependencies section of the package.json file unfortunately

stevefan1999-personal · 2023-07-05T16:26:18Z

The node Oracledb package can't be installed under ARM platforms (eg any modern mac), so this is not something we can add to the dependencies section of the package.json file unfortunately

The latest 6.x version of oracledb implemented a pure JS driver which I think should be out of the box compatible. I did not see any breaking changes or migration guide so I assumed they have backward compatibility.

By default, the node-oracledb driver is a pure JavaScript module that directly connects to Oracle Database 12.1 or later. This ‘Thin’ mode does not need Oracle Client libraries. The database can be on the same machine as Node.js, or it can be remote.

I don't think we used any features of thick mode? Far as I know we are just using some basic CRUD and it should be fine. I just can't find a reliable way to test it.

I tried to manually run the blackbox test but I see some dependencies issue:

@stevefan1999-personal ➜ /workspaces/directus (patch-2) $ TEST_DB=oracle pnpm run test:blackbox

> directus-monorepo@ test:blackbox /workspaces/directus
> pnpm --filter directus deploy --prod dist && pnpm --filter tests-blackbox test

Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: /workspaces/.pnpm-store/v3
  Virtual store is at:             dist/node_modules/.pnpm
api                                      |  WARN  deprecated json2csv@5.0.7
api                                      |  WARN  deprecated @types/keyv@4.2.0
app                                      |  WARN  deprecated @joeattardi/emoji-button@4.6.4
directus                                 |  WARN  deprecated consolidate@0.15.1
packages/extensions-sdk                  |  WARN  deprecated @types/cssnano@5.1.0
packages/update-check                    |  WARN  deprecated @npmcli/move-file@1.1.2
api                                      |  WARN  deprecated request@2.88.2
packages/specs                           |  WARN  deprecated superagent@7.1.6
api                                      |  WARN  deprecated har-validator@5.1.5
api                                      |  WARN  deprecated uuid@3.4.0
packages/extensions-sdk                  |  WARN  deprecated stable@0.1.8
.                                        |    +1139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 2631, reused 2573, downloaded 25, added 1139, done
 WARN  Failed to create bin at /workspaces/directus/dist/node_modules/.pnpm/vite@4.3.7_@types+node@20.2.0_sass@1.62.1/node_modules/vite/node_modules/.bin/sass. The source file at /workspaces/directus/dist/node_modules/.pnpm/sass@1.62.1/node_modules/sass/sass.js does not exist.
 WARN  Failed to create bin at /workspaces/directus/packages/dist/node_modules/.bin/markdown-it. The source file at /workspaces/directus/dist/node_modules/.pnpm/markdown-it@12.3.2/node_modules/markdown-it/bin/markdown-it.js does not exist.
 WARN  Failed to create bin at /workspaces/directus/dist/node_modules/.bin/eslint. The source file at /workspaces/directus/dist/node_modules/.pnpm/eslint@8.40.0/node_modules/eslint/bin/eslint.js does not exist.
 WARN  Failed to create bin at /workspaces/directus/dist/node_modules/.bin/prettier. The source file at /workspaces/directus/dist/node_modules/.pnpm/prettier@2.8.8/node_modules/prettier/bin-prettier.js does not exist.
 WARN  Issues with peer dependencies found
api
└─┬ @directus/extensions-sdk 10.1.5
  └─┬ rollup-plugin-styles 4.0.0
    └── ✕ unmet peer rollup@^2.63.0: found 3.22.0 in @directus/extensions-sdk

app
└─┬ @directus/extensions-sdk 10.1.5
  └─┬ rollup-plugin-styles 4.0.0
    └── ✕ unmet peer rollup@^2.63.0: found 3.22.0 in @directus/extensions-sdk

directus
└─┬ @directus/api 12.0.2
  └─┬ @directus/extensions-sdk 10.1.5
    └─┬ rollup-plugin-styles 4.0.0
      └── ✕ unmet peer rollup@^2.63.0: found 3.22.0 in @directus/extensions-sdk

packages/create-directus-extension
└─┬ @directus/extensions-sdk 10.1.5
  └─┬ rollup-plugin-styles 4.0.0
    └── ✕ unmet peer rollup@^2.63.0: found 3.22.0 in @directus/extensions-sdk

packages/extensions-sdk
└─┬ rollup-plugin-styles 4.0.0
  └── ✕ unmet peer rollup@^2.63.0: found 3.22.0

> tests-blackbox@ test /workspaces/directus/tests/blackbox
> jest

Error: Jest: Got error running globalSetup - /workspaces/directus/tests/blackbox/setup/setup.ts, reason: [TSError: common/functions.ts:5:28 - error TS2307: Cannot find module '@directus/types' or its corresponding type declarations.

5 import type { Query } from '@directus/types';
                             ~~~~~~~~~~~~~~~~~]
    at runGlobalHook (/workspaces/directus/node_modules/.pnpm/@jest+core@29.5.0/node_modules/@jest/core/build/runGlobalHook.js:117:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async runJest (/workspaces/directus/node_modules/.pnpm/@jest+core@29.5.0/node_modules/@jest/core/build/runJest.js:327:5)
    at async _run10000 (/workspaces/directus/node_modules/.pnpm/@jest+core@29.5.0/node_modules/@jest/core/build/cli/index.js:344:7)
    at async runCLI (/workspaces/directus/node_modules/.pnpm/@jest+core@29.5.0/node_modules/@jest/core/build/cli/index.js:199:3)
    at async Object.run (/workspaces/directus/node_modules/.pnpm/jest-cli@29.5.0/node_modules/jest-cli/build/run.js:124:37)
/workspaces/directus/tests/blackbox:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  tests-blackbox@ test: `jest`
Exit status 1
 ELIFECYCLE  Command failed with exit code 1.

I ran this under a codespace. Seems like the test did not see the linked workspace member somehow.

rijkvanzanten · 2023-07-06T09:14:53Z

The latest 6.x version of oracledb implemented a pure JS driver which I think should be out of the box compatible. I did not see any breaking changes or migration guide so I assumed they have backward compatibility.

Fantastic!! That's news to me 😄 Glad to hear that incredibly annoying requirement on native binaries that may or may not exist is a thing of the past. In that case, forget what I said before 🙂

.github/workflows/blackbox-main.yml

stevefan1999-personal · 2023-07-10T03:21:24Z

Now we need to run the blackbox test...preferrably before merging the PR.

stevefan1999-personal · 2023-07-10T07:17:23Z

✖ Bootstrap databases and start servers [FAILED: Directus-oracle bootstrap failed: 
 Error: select * from "directus_fields" where "interface" = :1 - ORA-01002: fetch out of sequence
    at Protocol._processMessage (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/thin/protocol/protocol.js:167:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ThinConnectionImpl.execute (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/thin/connection.js:802:7)
    at async Connection.execute (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/connection.js:861:16)
    at async Connection.<anonymous> (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/util.js:162:14) {
  offset: 0,
  errorNum: 1002,
  code: 'ORA-01002'
}
]

Hmm...ORA-01002: fetch out of sequence tips (dba-oracle.com)

stevefan1999-personal · 2023-07-10T07:26:42Z

https://github.com/stevefan1999-personal/directus/blob/8c23245b47394a978888e7c502630c691666c0ab/api/src/database/migrations/20210802A-replace-groups.ts#L6-L7
I'm not sure if this is the offending statement...

stevefan1999-personal · 2023-07-10T07:55:28Z

nektos/act: Run your GitHub Actions locally 🚀

Well, at least this still runs on my Github workspace, and I enabled only Oracle vendor, I'm looking forward to see it being reproduced in a short time

rijkvanzanten

Change LGTM, but lets see if the test will pass now (no clue why it shouldn't after this change, but oracle has surprised me in the past)

rijkvanzanten · 2023-07-24T19:40:41Z

Hmm same problem again. @aidenfoxx ever seen this issue fly by before? Kinda surprised it only started throwing this after upgrading the connection library

Error: Jest: Got error running globalSetup - /home/runner/work/directus/directus/tests/blackbox/setup/setup.ts, reason: Error: Directus-oracle bootstrap failed: 
 Error: select * from "directus_fields" where "interface" = :1 - ORA-01002: fetch out of sequence
    at Protocol._processMessage (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/thin/protocol/protocol.js:167:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ThinConnectionImpl.execute (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/thin/connection.js:802:7)
    at async Connection.execute (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/connection.js:861:16)
    at async Connection.<anonymous> (/home/runner/work/directus/directus/dist/node_modules/.pnpm/oracledb@6.0.2/node_modules/oracledb/lib/util.js:162:14) {
  offset: 0,
  errorNum: 1002,
  code: 'ORA-01002'
}

aidenfoxx · 2023-07-24T20:02:42Z

I've not seen it before. Could be a race condition, if other migrations are still being brought up on the Oracle side for some reason? Only thing I can think of that would cause such a basic query to fail. 🤷

licitdev · 2023-07-25T19:08:01Z

I wonder if the error is due to mismatched package versions, as Knex is using oracledb@5.5.0, pending a dependency bump. 🤔

Ref: knex/knex#5630, knex/knex#5615

stevefan1999-personal · 2023-09-09T16:34:34Z

All cleared.

Test Suites: 36 passed, 36 total
Tests:       9968 passed, 9968 total
Snapshots:   0 total
Time:        1778.646 s
Ran all test suites.
❯ Stop Directus servers
❯ OracleDB
✔ OracleDB
✔ Stop Directus servers

@rijkvanzanten I think it is good. Let's merge it!

stevefan1999-personal · 2023-09-11T03:06:09Z

@paescuj sorry to bother but do you think we need to have a way to let PRs to test the main workflow so that they can be safely merged. I think we can limit certain action workflows to get permission from maintainers first before launching.

paescuj · 2023-09-11T07:58:33Z

It seems like https://github.com/oracle/node-oracledb/releases/tag/v6.1.0 and oracle/node-oracledb#1565 was indeed the blocking factor and now it is fixed a week ago now, we can finally get this PR moving on.

Good news 😃

@rijkvanzanten I think it is good. Let's merge it!

Since we're using oracledb via Knex, we should wait for official support (knex/knex#5615). This should land soon, Node.js 20 will be LTS from October 24.

@paescuj sorry to bother but do you think we need to have a way to let PRs to test the main workflow so that they can be safely merged. I think we can limit certain action workflows to get permission from maintainers first before launching.

I don't think this is necessary, it's usually sufficient to run only the SQLite test in PRs. Exceptions are rare and can be handled manually 👍

stevefan1999-personal · 2023-09-11T08:11:01Z

@paescuj

Since we're using oracledb via Knex, we should wait for official support (knex/knex#5615). This should land soon, Node.js 20 will be LTS from October 24.

My concern is that the upstream Knex are trying to upgrade the version to 6.1.0 (as evidenced here: knex/knex#5675). The workflow for Oracle tests seems fine, but if you look closely, test errors are deliberately ignored. From my own prediction I'd be safe to assume at least 10% of them are supposed to be failing. Still, our integration tests here at Directus shows there is no problem whatsoever (all integration tests with Oracle Driver 6.1.0 passed, with Knex version compatibility bypassed thanks to semver).

stevefan1999-personal · 2023-09-12T08:49:27Z

@paescuj Bump oracledb from 5.5.0 to 6.1.0 by dependabot[bot] · Pull Request #5675 · knex/knex (github.com) is merged.

rijkvanzanten · 2023-09-13T19:26:32Z

heyyyy exciting stuff! So just to confirm, we're awaiting a new Knex release and then it's time to ship it?! ⛵

paescuj · 2023-09-13T20:08:28Z

heyyyy exciting stuff! So just to confirm, we're awaiting a new Knex release and then it's time to ship it?! ⛵

That would be my preferred way ⛵😃

stevefan1999-personal · 2023-11-23T06:09:09Z

@paescuj they have released 3.0.0 which included oracledb 6.1.0, mind merging this?

paescuj · 2023-11-23T09:39:56Z

@paescuj they have released 3.0.0 which included oracledb 6.1.0, mind merging this?

Yeah, I'll look into this soon 👍

This reverts commit 5749b20.

This reverts commit 0d8e632.

paescuj

Thanks @stevefan1999-personal!

stevefan1999-personal · 2023-12-18T14:37:17Z

@paescuj much appreciated for the follow ups!

Update package.json

ad5b841

Update contributors.yml

d96b705

Update lockfiles

c7af798

Remove oracle instant client with new thin client instead

eb6d510

stevefan1999-personal changed the title ~~Add oracledb to api/package.json~~ Add the latest version of oracledb to api/package.json Jul 5, 2023

rijkvanzanten added Package: @directus/api 🍰 Improvement labels Jul 6, 2023

rijkvanzanten requested changes Jul 6, 2023

View reviewed changes

.github/workflows/blackbox-main.yml Outdated Show resolved Hide resolved

rijkvanzanten self-assigned this Jul 6, 2023

stevefan1999-personal and others added 2 commits July 7, 2023 13:32

remove oracle client

fb5dab4

Merge branch 'main' into patch-2

978c342

Test all DB vendors

8c23245

rijkvanzanten removed their assignment Jul 10, 2023

Merge branch 'main' into patch-2

89ce10d

rijkvanzanten approved these changes Jul 24, 2023

View reviewed changes

stevefan1999-personal added 2 commits August 5, 2023 01:15

Merge branch 'main' into patch-2

40a6d79

Merge branch 'main' into patch-2

a4042fc

stevefan1999-personal added 2 commits September 11, 2023 10:53

Merge branch 'main' into patch-2

8aa06b0

Update blackbox-main.yml

c305ed5

Merge branch 'main' into patch-2

4c668c9

Merge branch 'main' into patch-2

c23dae6

Merge branch 'main' into patch-2

07bb64a

stevefan1999-personal added 2 commits September 18, 2023 13:33

Merge branch 'main' into patch-2

a3e1cb7

Merge branch 'main' into patch-2

9eb6a5f

paescuj added 8 commits December 16, 2023 02:09

Merge branch 'main' into patch-2

30b0763

Update oracledb to 6.2.0

d83da63

Temporarily activate the blackbox tests

0d8e632

Test oracledb only

5749b20

Merge branch 'main' into patch-2

5a75007

Add changeset

8dfc859

Revert "Test oracledb only"

3e4bc8e

This reverts commit 5749b20.

Revert "Temporarily activate the blackbox tests"

ea36f5b

This reverts commit 0d8e632.

paescuj approved these changes Dec 18, 2023

View reviewed changes

paescuj enabled auto-merge (squash) December 18, 2023 12:11

paescuj merged commit 7103277 into directus:main Dec 18, 2023
4 checks passed

github-actions bot added this to the Next Release milestone Dec 18, 2023

github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the latest version of oracledb to api/package.json #19067

Add the latest version of oracledb to api/package.json #19067

stevefan1999-personal commented Jul 5, 2023 •

edited

Loading

changeset-bot bot commented Jul 5, 2023 •

edited

Loading

stevefan1999-personal commented Jul 5, 2023 •

edited

Loading

stevefan1999-personal commented Jul 5, 2023

rijkvanzanten commented Jul 5, 2023

stevefan1999-personal commented Jul 5, 2023 •

edited

Loading

rijkvanzanten commented Jul 6, 2023

stevefan1999-personal commented Jul 10, 2023

stevefan1999-personal commented Jul 10, 2023

stevefan1999-personal commented Jul 10, 2023

stevefan1999-personal commented Jul 10, 2023

rijkvanzanten left a comment

rijkvanzanten commented Jul 24, 2023 •

edited

Loading

aidenfoxx commented Jul 24, 2023 •

edited

Loading

licitdev commented Jul 25, 2023

stevefan1999-personal commented Sep 9, 2023

stevefan1999-personal commented Sep 11, 2023

paescuj commented Sep 11, 2023

stevefan1999-personal commented Sep 11, 2023

stevefan1999-personal commented Sep 12, 2023

rijkvanzanten commented Sep 13, 2023

paescuj commented Sep 13, 2023

stevefan1999-personal commented Nov 23, 2023

paescuj commented Nov 23, 2023

paescuj left a comment

stevefan1999-personal commented Dec 18, 2023

	COPY pnpm-lock.yaml .
	RUN pnpm fetch
	COPY . .
	RUN pnpm install --recursive --offline --frozen-lockfile

	RUN : \
	&& npm_config_workspace_concurrency=1 pnpm run build \
	&& pnpm --filter directus deploy --prod dist \
	&& cd dist \
	&& pnpm pack \
	&& tar -zxvf *.tgz package/package.json \
	&& mv package/package.json package.json \
	&& rm -r *.tgz package \
	&& mkdir -p database extensions uploads \
	;

Add the latest version of oracledb to api/package.json #19067

Add the latest version of oracledb to api/package.json #19067

Conversation

stevefan1999-personal commented Jul 5, 2023 • edited Loading

changeset-bot bot commented Jul 5, 2023 • edited Loading

🦋 Changeset detected

stevefan1999-personal commented Jul 5, 2023 • edited Loading

stevefan1999-personal commented Jul 5, 2023

rijkvanzanten commented Jul 5, 2023

stevefan1999-personal commented Jul 5, 2023 • edited Loading

rijkvanzanten commented Jul 6, 2023

stevefan1999-personal commented Jul 10, 2023

stevefan1999-personal commented Jul 10, 2023

stevefan1999-personal commented Jul 10, 2023

stevefan1999-personal commented Jul 10, 2023

rijkvanzanten left a comment

Choose a reason for hiding this comment

rijkvanzanten commented Jul 24, 2023 • edited Loading

aidenfoxx commented Jul 24, 2023 • edited Loading

licitdev commented Jul 25, 2023

stevefan1999-personal commented Sep 9, 2023

stevefan1999-personal commented Sep 11, 2023

paescuj commented Sep 11, 2023

stevefan1999-personal commented Sep 11, 2023

stevefan1999-personal commented Sep 12, 2023

rijkvanzanten commented Sep 13, 2023

paescuj commented Sep 13, 2023

stevefan1999-personal commented Nov 23, 2023

paescuj commented Nov 23, 2023

paescuj left a comment

Choose a reason for hiding this comment

stevefan1999-personal commented Dec 18, 2023

stevefan1999-personal commented Jul 5, 2023 •

edited

Loading

changeset-bot bot commented Jul 5, 2023 •

edited

Loading

stevefan1999-personal commented Jul 5, 2023 •

edited

Loading

stevefan1999-personal commented Jul 5, 2023 •

edited

Loading

rijkvanzanten commented Jul 24, 2023 •

edited

Loading

aidenfoxx commented Jul 24, 2023 •

edited

Loading