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

WIP - Merge unee t ins improvements - Phase 1 #892

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

franck-boullier
Copy link
Member

@franck-boullier franck-boullier commented Feb 6, 2020

Overview:

This is to make sure we can
1- Deploy the frontend (fix deployment bug identified on 06/02/2020)
2- Get some of the improvements we made in the Unee-T INS fork
3- Start working on using AWS Parameter Store variables instead of hardcoded values to make this more versatile.

Issue we MUST fix:

bcrypt fails at the install script at node-gyp rebuild

Warning:

This branch will NOT deploy with TRAVIS CI UNLESS all the settings that Travis CI needs are configured and correctly set in the Travis Setting for each environment!

Kai Hendry and others added 26 commits December 29, 2019 13:48
Update mongo connect for Mongo 4.0 DB
Update mongo connect for Mongo 4.0 DB
* create new branch

* Change some hardcoded to environment variable

* Add config to use travis ci for dev

* check deploy dev

* check deploy dev

* check deploy dev

* check deploy dev

* Deploy with full steps

* change build branch Master

* remove sudo and skip_cleanup in .travis.yml

* remove hardcode in AWS-docker-compose.yml file

* add a comment to buildspec.yml

* test the meteor build step

* change bcrypt from 1.0.3 (which is not supported in node12) to 3.0.6

* change deploy branch for dev to master
* Use clear variable names

* Check variables

* update variable in aws-env.dev

* change build dev on branch master
* replace confusing variable name with a better name

* update aws-env.[STAGE] for DEMO and PROD

* fix linting

* rename variable CLOUDINARY_URL to CLOUDINARY_API_ENDPOINT
This is related to issue #17: we need to use redeploy and use the correct value for the AWS variable `CLOUDINARY_CLOUD_NAME`
WIP - Import changes and improvements from Unee-T INS
@franck-boullier
Copy link
Member Author

More info:

The deployment error is probably linked to a change we have to make in the package.json file

on line 22, we have replaced
"bcrypt": "^1.0.3",
with
"bcrypt": "^3.0.6"

Mongo DB version:

In Unee-T INS we are using a more recent version of the Mongo DB.
We need to evaluate if this has an impact (it probably has: connection string is different)

@kaihendry
Copy link
Contributor

The Docker build fails https://media.dev.unee-t.com/2020-02-07/merge_unee-t_ins_improvements.txt which appears to be some fibers dependency. I think there is a crazy balancing act between:

  1. Node version
  2. Bcrypt version https://github.com/kelektiv/node.bcrypt.js#version-compatibility
  3. Fibers version
  4. Meteor version

Sidenote: Bit concerning that frontend master is also breaking: https://media.dev.unee-t.com/2020-02-07/frontend-master.txt ... I'm not sure how this happened.

I can see a bcrypt error here c972242 from Dec 3rd. This should have been addressed earlier.

@kaihendry
Copy link
Contributor

kaihendry commented Feb 7, 2020

I saw your latest commits to https://github.com/unee-t/frontend/blob/merge_unee-t_ins_improvements/.travis.yml to put back Docker commands instead of https://github.com/unee-t/frontend/blob/master/buildspec.yml

I wish this was a separate PR titled "Deprecate CodeBuild in favour of all building happening in Travis"

Then this might be sane to follow.

Anyway, big issue with CodeBuild is that it doesn't integrate with Github well at all. So it is a good idea to move back to Travis imo btw.

kaihendry pushed a commit that referenced this pull request Feb 7, 2020
#750 which I think solves
#892 (comment)
because it locks down to .meteor/release
@franck-boullier
Copy link
Member Author

franck-boullier commented Feb 9, 2020

Trying to look into why Docker build is nor working with Node Js 12.
There was a similar looking issue here: laverdet/node-fibers#409.
Need to check if the fix implemented will also work for us.
See 6bbc688

kaihendry pushed a commit that referenced this pull request Feb 10, 2020
#750 which I think solves
#892 (comment)
because it locks down to .meteor/release
@franck-boullier franck-boullier changed the title Merge unee t ins improvements - Phase 1 WIP - Merge unee t ins improvements - Phase 1 Feb 10, 2020
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.

3 participants