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

Instructions in code-setup fails to build on darwin #134

Closed
psx95 opened this issue Jun 19, 2023 · 10 comments · Fixed by #142
Closed

Instructions in code-setup fails to build on darwin #134

psx95 opened this issue Jun 19, 2023 · 10 comments · Fixed by #142
Labels
bug Something isn't working C4GT Community - Legacy C4GT good first issue Good for newcomers help wanted Extra attention is needed P2

Comments

@psx95
Copy link

psx95 commented Jun 19, 2023

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 [email protected]: 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

Category

Bug fix

Sub Category

Project Setup

Domain

Others

@techsavvyash techsavvyash added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers C4GT P2 C4GT Community - Legacy labels Jun 20, 2023
@Deekshithrathod
Copy link
Contributor

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
Copy link
Contributor

-┬ [email protected]
│ ├── [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ └── UNMET OPTIONAL DEPENDENCY [email protected]

@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
Copy link
Collaborator

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
Copy link
Contributor

@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
Copy link
Collaborator

techsavvyash commented Jul 8, 2023

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

@Deekshithrathod
Copy link
Contributor

@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
Copy link
Collaborator

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

@Deekshithrathod
Copy link
Contributor

@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
Copy link

c4gt-community-support bot commented Jul 13, 2023

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
Copy link
Contributor

@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.

@techsavvyash techsavvyash linked a pull request Jul 24, 2023 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C4GT Community - Legacy C4GT good first issue Good for newcomers help wanted Extra attention is needed P2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants