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

Relocate persistent data files into a user profile. #663

Merged
merged 1 commit into from Feb 27, 2018

Conversation

mrstegeman
Copy link
Contributor

@mrstegeman mrstegeman commented Feb 12, 2018

These need to be moved out of the gateway tree for data persistence
when using Docker. This will also allow users to easily switch
profiles, if desired.

Z-Wave logs and such will be handled at the add-on level.

@ghost ghost assigned mrstegeman Feb 12, 2018
@ghost ghost added the review label Feb 12, 2018
openssl genrsa -out ssl/privatekey.pem 2048
openssl req -new -sha256 -key ssl/privatekey.pem -out ssl/csr.pem -subj '/CN=www.toizom.com/O=MozillaIoT Gateway/C=US'
openssl x509 -req -in ssl/csr.pem -signkey ssl/privatekey.pem -out ssl/certificate.pem
export SSL_DIR="${HOME}/mozilla-iot/data/ssl"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be exported?

Copy link
Contributor

Choose a reason for hiding this comment

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

And there really needs to be a way to override this. My mozilla-iot directory isn't in my HOME directory, it's in my Dropbox folder.

@@ -2,7 +2,7 @@

SCRIPTDIR="$(dirname ""$0"")"

if [ ! -f "ssl/certificate.pem" ]; then
if [ ! -f "$HOME/mozilla-iot/data/ssl/certificate.pem" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably have an environment variable like MOZIOT_HOME default to ${HOME}/mozilla-iot but allow it to be override. All of the bash scripts should probably use something like:

: ${MOZIOT_HOME:="${HOME}/mozilla-iot"}

or the slightly more verbose, but perhaps more easily understood

MOZIOT_HOME="${MOZIOT_HOME:=${HOME}/mozilla-iot}"

and then use ${MOZIOT_HOME} everywhere

src/app.js Outdated

if (fs.existsSync('db.sqlite3')) {
fs.renameSync('db.sqlite3', dbPath);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this deserves a comment to explain why its here (i.e. migration)

@benfrancis
Copy link
Member

Can we have a quick discussion about this in our meeting today after planning the 0.4 release and before landing? It seems like a modestly large architectural change that warrants a group discussion.

@codecov-io
Copy link

codecov-io commented Feb 13, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@5b90d51). Click here to learn what that means.
The diff coverage is 71.28%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #663   +/-   ##
========================================
  Coverage          ?   80.2%           
========================================
  Files             ?      82           
  Lines             ?    3354           
  Branches          ?     426           
========================================
  Hits              ?    2690           
  Misses            ?     618           
  Partials          ?      46
Impacted Files Coverage Δ
config/test.js 100% <ø> (ø)
src/tunnel_setup.js 86.66% <100%> (ø)
src/ssltunnel.js 30.76% <100%> (ø)
config/default.js 100% <100%> (ø)
src/db.js 82.35% <100%> (ø)
src/controllers/uploads_controller.js 75.86% <100%> (ø)
src/controllers/settings_controller.js 30.9% <25%> (ø)
src/user-profile.js 67.1% <67.1%> (ø)
src/app.js 66.89% <80%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b90d51...31f26f4. Read the comment docs.

@mrstegeman
Copy link
Contributor Author

@benfrancis Sure thing.

@mrstegeman mrstegeman force-pushed the relocate-persistent-files branch 2 times, most recently from 73f8aa3 to 88c58a3 Compare February 14, 2018 17:26
@mrstegeman mrstegeman changed the title Relocate persistent data files. Relocate persistent data files into a user profile. Feb 14, 2018
@mrstegeman mrstegeman dismissed dhylands’s stale review February 14, 2018 17:27

Please review again.

@mrstegeman mrstegeman force-pushed the relocate-persistent-files branch 2 times, most recently from 6f2997e to d2f3241 Compare February 21, 2018 19:23
These need to be moved out of the gateway tree for data persistence
when using Docker. This will also allow users to easily switch
profiles, if desired.
@mrstegeman
Copy link
Contributor Author

@benfrancis are we ready to move forward with this, or are there still outstanding issues?

Copy link
Member

@benfrancis benfrancis left a comment

Choose a reason for hiding this comment

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

Yes, this is OK with me. Probably a good idea!

},
bcryptRounds: 2
bcryptRounds: 2,
Copy link
Member

Choose a reason for hiding this comment

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

What's with all the extra commas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not JSON, so the trailing commas make maintenance easier.

@@ -181,6 +183,7 @@ $ jest src/test/{test-name}.js
* **`router.js`** - Routes app URLs to controllers
* **`ssltunnel.js`** - Utilities to determine state of tunnel and manage the PageKite process
* **`tunnel_setup.js`** - Express middleware to determine if the tunnel is set up
* **`user-profile.js`** - Manages persistent user data
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it's really a user profile if it's shared by all users, but to be honest I can't think of a better name.

@mrstegeman mrstegeman merged commit 9e26b79 into WebThingsIO:master Feb 27, 2018
@ghost ghost removed the review label Feb 27, 2018
@mrstegeman mrstegeman deleted the relocate-persistent-files branch February 27, 2018 20:23
@mrstegeman mrstegeman added this to Done in WebThings Gateway Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants