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

bump @nestjs/axios from 0.1.0 to 3.0.0 #661

Merged
merged 12 commits into from
Aug 16, 2023

Conversation

bpedersen2
Copy link
Contributor

Bump axios to 3.0.0

adjust tests to axios 3

supersedes: #568

@bpedersen2
Copy link
Contributor Author

The last commit fixes:

> nest build

src/datasets/dto/update-derived-dataset.dto.ts:16:12 - error TS2612: Property 'pid' will overwrite the base property in 'CreateDerivedDatasetDto'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.

16   readonly pid?: string;
              ~~~
src/datasets/dto/update-raw-dataset.dto.ts:16:12 - error TS2612: Property 'pid' will overwrite the base property in 'CreateRawDatasetDto'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.

16   readonly pid?: string;
              ~~~
src/origdatablocks/dto/create-origdatablock.dto.ts:66:12 - error TS2612: Property 'ownerGroup' will overwrite the base property in 'OwnableDto'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.

66   readonly ownerGroup: string;

@bpedersen2
Copy link
Contributor Author

To use axios 3, I needed to set the target to es2022 in tsconfig.json and upgrade the containers to node-18.

@bpedersen2
Copy link
Contributor Author

Still need to find out why the unit and api tests now are hanging ( the e2e test runs successful)

@bpedersen2 bpedersen2 force-pushed the axios-3 branch 4 times, most recently from 64cdd57 to 06c0744 Compare August 11, 2023 10:27
@bpedersen2
Copy link
Contributor Author

The tests are passing now.

@bpedersen2
Copy link
Contributor Author

Note that this a breaking change that requires to use at least node 18.

@nitrosx
Copy link
Member

nitrosx commented Aug 11, 2023

Waiting for a day, to see if the other reviewers wants to provide additional feedback.

@nitrosx
Copy link
Member

nitrosx commented Aug 11, 2023

@bpedersen2 should we close/delete #568?

@bpedersen2
Copy link
Contributor Author

Waiting for a day, to see if the other reviewers wants to provide additional feedback.

Yes, that would be good (that's why tried to put some of the more active users in, maybe you can add one from PSI as well?)

@nitrosx
Copy link
Member

nitrosx commented Aug 14, 2023

@bpedersen2 I'm ready to merge your PR.
Would you be able to solve the conflict on the file package-lock.json?
Thanks

@bpedersen2
Copy link
Contributor Author

@bpedersen2 I'm ready to merge your PR. Would you be able to solve the conflict on the file package-lock.json? Thanks

Yes, I'll push an update

Change-Id: Ib55be9b468f342fdd0ac02fb96d1af0a3739e80b
Change-Id: Id588413f7783831e3548daa973ca6bcedd036c11
Change-Id: Ic822ec87e03594142bebf8ff9bdc700a1e046437
the config service seems not to be ready otherwise

Change-Id: I76bd8512468fbbb48a2275ddcfccc0cddf8795a8
Change-Id: I18ee85da01954435f5deee8f375c64b1b4d21854
Change-Id: I19aca87b909a909fbe8e39b36891a8b651ceba4c
Change-Id: I0b7c8fca2fb91560e8d92e3758e1d194c15da864
Change-Id: Iec81302e71cce91ae5537a9b03d442d87569ddbb
Change-Id: I180b732b8063d1d3d8a653410fc23ab717fd203e
Change-Id: I6fd4ccaca4876454647ddd7d640b845f28fbb6cc
Change-Id: Ibbb4ab85830d9449ce6e0bb50824fefce51869cb
@nitrosx nitrosx merged commit 0560089 into SciCatProject:master Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants