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

Doesn't check that binaries have been already downloaded #84

Closed
ChronicusUA opened this issue Oct 4, 2018 · 15 comments
Closed

Doesn't check that binaries have been already downloaded #84

ChronicusUA opened this issue Oct 4, 2018 · 15 comments

Comments

@ChronicusUA
Copy link
Contributor

Hi,

I got an issue that it is downloading binaries twice: after npm install and when instance is going to run.

I have a problem in this case:
In my CI/CD job I run inside docker container npm i and npm test commands and it always download binaries at least 1 time.
Please add checking that binaries have been already downloaded.

@nodkz
Copy link
Owner

nodkz commented Oct 4, 2018

It looks like you are using some exact mongodb version in your tests.

Please add MONGOMS_VERSION var to your env variables in CI config.

Related to #76

@nodkz
Copy link
Owner

nodkz commented Oct 4, 2018

By default downloaded latest version

@ChronicusUA
Copy link
Contributor Author

I am using the env variable in my Dockerfile.
Part of my Dockerfile:

## Dockerfile
FROM node:8.9.3-slim

ENV MONGOMS_VERSION=3.4.4

COPY . /my-app/source

RUN rm -rf ./node_modules

RUN npm install --unsafe-perm --loglevel=warn
# Here we see downloading process

RUN npm test
#And here we see downloading process

I took a look on code and seems like in MongoBinary you create cache as empty object on startup without checking any existing downloads. And then the version is put to the cache object only after downloading.
But when docker container is running it doesn't have run MongoInstance and the cache object is still empty and it is a reason why it tries to download it again.

@nodkz
Copy link
Owner

nodkz commented Oct 4, 2018

Can you make PR?

@AJRdev
Copy link
Collaborator

AJRdev commented Oct 11, 2018

@nodkz Is the env variable MONGOMS_DOWNLOAD_DIR not enough for this specific use case ?
By specifying it in the Dockerfile and providing the cache binary from gitlab to the path specified by MONGOMS_DOWNLOAD_DIR in the container.

The code in the getPath function of MongoBinary class using this env variable :

const defaultOptions = {
      downloadDir:
        process.env?.MONGOMS_DOWNLOAD_DIR || path.resolve(os.homedir(), '.mongodb-binaries'),
      platform: process.env?.MONGOMS_PLATFORM || os.platform(),
      arch: process.env?.MONGOMS_ARCH || os.arch(),
      version: process.env?.MONGOMS_VERSION || 'latest',
      debug:
        typeof process.env.MONGOMS_DEBUG === 'string'
          ? ['1', 'on', 'yes', 'true'].indexOf(process.env.MONGOMS_DEBUG.toLowerCase()) !== -1
          : false,
    };

@nodkz
Copy link
Owner

nodkz commented Oct 11, 2018

Need to meet MONGOMS_DOWNLOAD_DIR and MONGOMS_VERSION. These vars are used for storing binaries in some path.

So by default with npm install will be downloaded latest version. But if tests uses for example v3.6.4 it will be fownloaded again.

What I think, that this lib should have some bin script, which can be used in docker file:

  • firstly install npm
  • then run cmd for install other mongo version if needed

@AJRdev
Copy link
Collaborator

AJRdev commented Oct 12, 2018

@nodkz I've been trying to cache as well the mongdb binary download with Gitlab CI as well.
I've tried doing that by using the env variables you suggested (I'm using the lastest version of the mongodb binary so I didn't specified MONGOMS_VERSION), here is my gitlab ci file :

stages:
  - functionnal_tests

functionnal_tests:
 image: node:8
 stage: functionnal_tests
 variables:
  MONGOMS_DOWNLOAD_DIR: 'mongobinary'
 cache:
   paths:
   - mongobinary
   - node_modules
 script:
   - npm install
   - npm test

I've suceeded to cache the npm test command where the mongodb binary is downloaded to the path specified by MONGOMS_DOWNLOAD_DIR.
But it seems that on the npm install command the binary is not downloaded to the same path, it is downloaded to node_modules/mongodb-memory-server/ + MONGOMS_DOWNLOAD_DIR, here is the trace of the debug of mongdb-memory-server :

$ npm install

> mongodb-memory-server@2.4.3 postinstall /builds/api/node_modules/mongodb-memory-server
> node ./postinstall.js

mongodb-memory-server: checking MongoDB binaries cache...
Downloading MongoDB latest: 0 % (0mb / 84.5mb)
Downloading MongoDB latest: 52.2 % (44.1mb / 84.5mb)
mongodb-memory-server: binary path is /builds/api/node_modules/mongodb-memory-server/mongobinary/latest/mongod

Is this behaviour normal ?
Maybe the MongoBinary class is just appending a download path to the current directory ?
It would explain why my binary is downloaded in the node_modules directory since the npm install is done in that directory.
And is the binary download on the npm install command really useful ? Isn't the download on npm test when we create an instance of MongodbMemoryServer enough ?

@AJRdev
Copy link
Collaborator

AJRdev commented Oct 12, 2018

I found out the source of the problem it was because of the path I gave to MONGOMS_DOWNLOAD_DIR that was incorrect and using the current directory, I specified the path of the build directory of the gitlab pipeline and I suceeded to cache the npm install and npm test binary download :)

But still why do we need to download two times the mongodb binary in npm install and npm test and not only in the npm test command ?

@ChronicusUA
Copy link
Contributor Author

Sorry for delay, I've investigated it deeper and when I set version as latest it works correct but if I provide another version it downloads twice. Seems like it needs to delete version: 'latest' from postinstall.js anyway it will use it by default.
About previous comment of install only in the npm test command in my case it nice to have this feature but if you will change it please leave a possibility to install it in npm install command may be just add env variable MONGOMS_DOWNLOAD_ON_POSTINSTALL

ChronicusUA added a commit to ChronicusUA/mongodb-memory-server that referenced this issue Oct 17, 2018
@nodkz nodkz closed this as completed Oct 17, 2018
@nodkz
Copy link
Owner

nodkz commented Oct 17, 2018

Fix already on npm.

@ChronicusUA
Copy link
Contributor Author

Can I add one more PR for MONGOMS_DOWNLOAD_ON_POSTINSTALL env variable?

@nodkz
Copy link
Owner

nodkz commented Oct 18, 2018

It should download binary automatically on post install, cause many users don't know how to increase timeout limit for test runners. So better to download binaries on package install.

Why do you need to add MONGOMS_DOWNLOAD_ON_POSTINSTALL variable?
You have any problems right now? Binary did not download as expected?

@ChronicusUA
Copy link
Contributor Author

I found one more situation that I need to don't download in npm i. I would like to add env variable that will cancel this issue but leave it as it is by default. So in this case if you need to download in npm i you don't need to do anything, but with this variable you can avoid it.

@ChronicusUA
Copy link
Contributor Author

Another words I have 2 cases when I need to download in npm i and another one when I need to download it in npm test

@nodkz
Copy link
Owner

nodkz commented Oct 18, 2018

So it can be named MONGOMS_DISABLE_POSTINSTALL

Open PR!

@nodkz nodkz reopened this Oct 18, 2018
ChronicusUA added a commit to ChronicusUA/mongodb-memory-server that referenced this issue Oct 18, 2018
nodkz pushed a commit that referenced this issue Oct 18, 2018
…ry download on package install

* fix: Remove latest verion from postinstall.js (#84)

* feat: add MONGOMS_DOWNLOAD_ON_POSTINSTALL env variable

* fix: rename disable postinstall env vriable (#84)
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

No branches or pull requests

3 participants