Instructions in code-setup fails to build on darwin

psx95 commented 1 year ago

Description

The code setup and build instructions in code-setup doc fails to build on darwin.

Steps to reproduce:

Follow the build instructions on code-setup doc on a darwin machine.

Root Cause:

There seems to be an explicit dependency on the linux-x64 binary for node-polars npm dependency which causes the the npm install to look for os type "linux".

npm install Fails with the error. (yarn install fails with a similar error) -

npm ERR! code EBADPLATFORM
npm ERR! notsup Unsupported platform for nodejs-polars-linux-x64-gnu@0.7.2: wanted {"os":"linux","arch":"x64"} (current: {"os":"darwin","arch":"arm64"})
npm ERR! notsup Valid OS:    linux
npm ERR! notsup Valid Arch:  x64
npm ERR! notsup Actual OS:   darwin
npm ERR! notsup Actual Arch: arm64

Potential Fix:

The build succeeded after removing the mentioned dependency from package.json. This was verified by successfully running the tests via yarn test which succeeded after removing the dependency.

There already seems to be a dependency on nodejs-polars, which makes me think that explicit dependency on the linux-x64 binary for this package can be removed.

Additional Info:

Tested on a MacOS 12.5.1 running Apple Silicon (M1)

Goals

The goal is to fix the issue as outlined in the description either via the suggested fix or some other way.

Project

cQube-ingestion

Organisation Name

Samagra | Transforming Governance

Mentor(s)

@techsavvyash @ChakshuGautam

Technical Skills Needed

NodeJS, NPM

Complexity

Low

Sub Category

Project Setup

Domain

Others

Deekshithrathod commented 1 year ago

I reproduced the issue & tried out removing the dependency too.. from the package.json file it works fine after the removal, but how do we ensure that the removal of this dependency doesn't affect other things? @techsavvyash

Deekshithrathod commented 1 year ago

-┬ nodejs-polars@0.7.2
│ ├── nodejs-polars-darwin-arm64@0.7.2
│ ├── UNMET OPTIONAL DEPENDENCY nodejs-polars-darwin-x64@0.7.2
│ ├── UNMET OPTIONAL DEPENDENCY nodejs-polars-linux-arm64-gnu@0.7.2
│ ├── UNMET OPTIONAL DEPENDENCY nodejs-polars-linux-x64-gnu@0.7.2
│ ├── UNMET OPTIONAL DEPENDENCY nodejs-polars-win32-ia32-msvc@0.7.2
│ └── UNMET OPTIONAL DEPENDENCY nodejs-polars-win32-x64-msvc@0.7.2

@techsavvyash @ChakshuGautam can you guys guide me here?

I wanted to know “Out of the 6 optional dependencies”, is one picked & installed based on the host hardware & OS by nodejs-polars? (and the other five are ignored?) Or should the developer add some configuration to handle this? or should the Devs manually modify the package.json as the OS & hardware change?

techsavvyash commented 1 year ago

I reproduced the issue & tried out removing the dependency too.. from the package.json file it works fine after the removal, but how do we ensure that the removal of this dependency doesn't affect other things? @techsavvyash

Hey @Deekshithrathod, For this, you should run the tests present, using the commands yarn test and yarn test:e2e. That should be enough I guess.

Deekshithrathod commented 1 year ago

@techsavvyash Alright, cool. Reached out to the maintainer of nodejs-polars. You can find his answer here.

I ran the test previously seems to work fine, none of them failed when I removed the Linux dependency. Will check again & raise a PR. Do I have to wait till it's assigned to me? or can I raise a PR immediately?

techsavvyash commented 1 year ago

Hey @Deekshithrathod, please go ahead and raise a PR. This stack overflow answer is highly appreciated. Thanks for the contribution.

Deekshithrathod commented 1 year ago

@techsavvyash Unit tests seem to run fine, but all end-to-end tests are failing, I don’t think the removal of nodejs-polars-linux caused this. Anyways, below is the log of the first test failure.

Nest can't resolve dependencies of the DatasetService (PrismaService, QueryBuilderService, EventService, ?). Please make sure that the argument DATABASE_POOL at index [3] is available in the RootTestModule context.

    Potential solutions:
    - Is RootTestModule a valid NestJS module?
    - If DATABASE_POOL is a provider, is it part of the current RootTestModule?
    - If DATABASE_POOL is exported from a separate @Module, is that module imported within RootTestModule?
      @Module({
        imports: [ /* the Module containing DATABASE_POOL */ ]
      })

      13 |
      14 |   beforeEach(async () => {
    > 15 |     const moduleFixture: TestingModule = await Test.createTestingModule({
         |                                          ^
      16 |       imports: [AppModule],
      17 |       providers: [
      18 |         EventService,

      at TestingInjector.lookupComponentInParentModules (../node_modules/@nestjs/core/injector/injector.js:241:19)
      at TestingInjector.resolveComponentInstance (../node_modules/@nestjs/core/injector/injector.js:194:33)
      at TestingInjector.resolveComponentInstance (../node_modules/@nestjs/testing/testing-injector.js:16:45)
      at resolveParam (../node_modules/@nestjs/core/injector/injector.js:116:38)
          at async Promise.all (index 3)
      at TestingInjector.resolveConstructorParams (../node_modules/@nestjs/core/injector/injector.js:131:27)
      at TestingInjector.loadInstance (../node_modules/@nestjs/core/injector/injector.js:57:13)
      at TestingInjector.loadProvider (../node_modules/@nestjs/core/injector/injector.js:84:9)
          at async Promise.all (index 4)
      at TestingInstanceLoader.createInstancesOfProviders (../node_modules/@nestjs/core/injector/instance-loader.js:47:9)
      at ../node_modules/@nestjs/core/injector/instance-loader.js:32:13
          at async Promise.all (index 1)
      at TestingInstanceLoader.createInstances (../node_modules/@nestjs/core/injector/instance-loader.js:31:9)
      at TestingInstanceLoader.createInstancesOfDependencies (../node_modules/@nestjs/core/injector/instance-loader.js:21:9)
      at TestingInstanceLoader.createInstancesOfDependencies (../node_modules/@nestjs/testing/testing-instance-loader.js:14:9)
      at TestingModuleBuilder.compile (../node_modules/@nestjs/testing/testing-module.builder.js:47:9)
      at Object.<anonymous> (app.e2e-spec.ts:15:42)

I’m completely new to nest & typescript, but from the inital glance I understood that some form of Dependency injection is happening here.

My assumption is that injection is happening without instantiating, I would appreciate it if you can point me to a source where I can find more about this. If you can provide some help directly that would be even better.

Injectable()
export class DatasetService {
  private readonly logger: Logger = new Logger(DatasetService.name);
  constructor(
    public prisma: PrismaService,
    private qbService: QueryBuilderService,
    private eventGrammarService: EventService,
    @Inject('DATABASE_POOL') private pool: Pool, // injected without instantiating
  ) {}

techsavvyash commented 1 year ago

Hey @Deekshithrathod, I suppose the e2e tests are broken right now. Please make sure the unit tests work and raise a PR. Thanks

Deekshithrathod commented 1 year ago

@techsavvyash although this is not really that important, still raised a PR. Lmk if you want me to make any more changes.

c4gt-community-support[bot] commented 1 year ago

Hi! Mandatory Details - The following details essential to submit tickets to C4GT Community Program are missing. Please add them!

Product Name - Please add a heading called Product Name and mention the name of the product below it.

Without these details, the ticket cannot be listed on the C4GT Community Listing.

Please update the ticket

Deekshithrathod commented 1 year ago

@techsavvyash I see that you've merged the changes, you should close this issue as well or should I do it from my end? (also, do I have the appropriate permissions to do that?). Also, I have another PR waiting for approval, that too.. is a simple one here if possible, please take a loot at it.

ChakshuGautam / cQube-ingestion