Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Minor fixes for next release (originally fixing yarn warning) #9

Merged
merged 4 commits into from
Aug 28, 2018

Conversation

rzr
Copy link
Contributor

@rzr rzr commented Aug 10, 2018

yarn failed to install and thus failed to install gateway dependencies,
to we relocate it to let it find gateway's package.json file.

Observed (silent) issue on docker 17.12.1-ce (Ubuntu-18.04) on x86_64:

npm WARN saveError ENOENT: no such file or directory, open '/home/node/package.json'
(...)
+ yarn@1.9.4
added 1 package in 0.447s
Cloning into 'intent-parser'...
Cloning into 'gateway'...
yarn install v1.9.4
[1/4] Resolving packages...
[1/4] Resolving packages...
[2/4] Fetching packages...
info fsevents@1.2.4: The platform "linux" is incompatible with this module.
(...)
[3/4] Linking dependencies...
[4/4] Building fresh packages...

Change-Id: I8737dfc28da8c8bf266de829725f3c09f62d9468
Signed-off-by: Philippe Coval p.coval@samsung.com

@mrstegeman
Copy link
Contributor

@rzr I intended for it to be installed in the home directory, rather than conflict with the gateway's own package.json. It's not a big deal if it can't be saved to /home/node/package.json, as yarn will still work just fine. Your log shows that yarn is running just fine.

@rzr
Copy link
Contributor Author

rzr commented Aug 10, 2018

OK I think I faced a bigger problem I will update PR eventually

@rzr
Copy link
Contributor Author

rzr commented Aug 10, 2018

With this change:
WebThingsIO/gateway#1260

I noticed that wpa-supplicant was missing I will replace this PR once everything is checked.

@mrstegeman
Copy link
Contributor

mrstegeman commented Aug 10, 2018

We just need to run the gateway without the --check-wifi flag.

@rzr rzr force-pushed the sandbox/rzr/review/master branch from 2384de7 to a1a749c Compare August 10, 2018 15:12
@rzr
Copy link
Contributor Author

rzr commented Aug 10, 2018

Ok so the change should be in runapp.sh ? I am trying this too

@mrstegeman
Copy link
Contributor

Yes, either that or we need a separate startup script.

rzr added a commit to TizenTeam/gateway that referenced this pull request Aug 10, 2018
To avoid unexpected behaviours, quit on error (set -e)

Change-Id: Ifbf6401b66afd71472f1b953a7aa4b57c0ecfd3f
Relate-to: WebThingsIO/gateway-docker#9
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this pull request Aug 10, 2018
To avoid unexpected behaviours, quit on error (set -e)

Change-Id: Ifbf6401b66afd71472f1b953a7aa4b57c0ecfd3f
Relate-to: WebThingsIO/gateway-docker#9
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@hobinjk
Copy link

hobinjk commented Aug 10, 2018

Is there any reason you don't want to use yarn start here? The way I see it run-app.sh is the script for running on a real production device. My main question would be if there's ever a case when you'd want to OTA upgrade a docker-based installation, because then using run-app.sh would be reasonable.

hobinjk pushed a commit to WebThingsIO/gateway that referenced this pull request Aug 10, 2018
To avoid unexpected behaviours, quit on error (set -e)

Change-Id: Ifbf6401b66afd71472f1b953a7aa4b57c0ecfd3f
Relate-to: WebThingsIO/gateway-docker#9
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@mrstegeman
Copy link
Contributor

yarn start is probably sufficient, with a log redirect. I plan on building and releasing an 0.5 image when I'm back in a couple weeks.

@rzr
Copy link
Contributor Author

rzr commented Aug 10, 2018

I'll be back in 1 week too, maybe I could add a couple of changes more but it's not critical

@rzr rzr force-pushed the sandbox/rzr/review/master branch from a1a749c to 20a0e1a Compare August 10, 2018 17:07
@rzr rzr changed the title Fix yarn failure to install (package.json lookup) Minor fixes for next release (orinally fixing yarn warning) Aug 10, 2018
@rzr rzr changed the title Minor fixes for next release (orinally fixing yarn warning) Minor fixes for next release (originally fixing yarn warning) Aug 10, 2018
Change-Id: I3e634f89b29baf42cfc02a56c0b13edda489af45
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this pull request Aug 21, 2018
To avoid unexpected behaviours, quit on error (set -e)

Change-Id: Ifbf6401b66afd71472f1b953a7aa4b57c0ecfd3f
Relate-to: WebThingsIO/gateway-docker#9
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr
Copy link
Contributor Author

rzr commented Aug 22, 2018

@mrstegeman beware @hobinjk might drop yarn support, can we just rely on NPM ?

WebThingsIO/gateway#1286

@mrstegeman
Copy link
Contributor

That shouldn't be a problem. I'll tackle it when necessary.

@hobinjk
Copy link

hobinjk commented Aug 22, 2018

@rzr The potential dropping of yarn support is very far in the future since it would require changing a significant amount of our build scripts. It's definitely not something that will happen soon enough to impact this PR

volumes:
- /root/mozilla-iot/gateway
ports:
- "8080:8080"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Port 4443 should also be added here.

services:
web:
build: .
command: /root/mozilla-iot/gateway/run-app.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/home/node/mozilla-iot/gateway/run-app.sh

build: .
command: /root/mozilla-iot/gateway/run-app.sh
volumes:
- /root/mozilla-iot/gateway
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/home/node/.mozilla-iot

Usage:

   docker-compose up

Change-Id: Iae65fe96fa64765cad0478ecd83e01c9439f0e18
Signed-off-by: Philippe Coval <p.coval@samsung.com>
Change-Id: Ide0cb0dae99cd207ef7af97963c36a1b1b8098d1
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr rzr force-pushed the sandbox/rzr/review/master branch from 20a0e1a to e7966c7 Compare August 27, 2018 20:21
@ghost ghost assigned mrstegeman Aug 28, 2018
@mrstegeman mrstegeman merged commit 85abdf8 into WebThingsIO:master Aug 28, 2018
@ghost ghost removed the review label Aug 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants