strongloop / strong-pm

deployer for node applications
http://strong-pm.io
Other
1k stars 71 forks source link

Make sqlite an optional PM dependency #336

Closed curtisr7 closed 8 years ago

curtisr7 commented 8 years ago

strongloop/strong-pm#299 updated strong-pm to use sqlite, but that breaks platforms / machines that don't have sqlite installed.

This PR makes the sqlite dependency optional and handles the missing dep at runtime if --json-file-db is provided.

sam-github commented 8 years ago

Lgtm

curtisr7 commented 8 years ago

@PeteStein sorry about the slow reply, but I'll take a look this afternoon.

Pete-St commented 8 years ago

Based on additional context with my specific problem, I think the only issue here is that the try/catch assumes that the only problem is that sqlite3 would be missing, but it seems like there are quite a few upstream problems with npm/sqlite3/node-gyp that are allowing for sqlite3 to be half-installed (module installed, but native components in the sqlite3/lib/binding folder not installed). There's a thread at node-gyp about the confusion about what user credentials are used when installing a module as root; I think I need to dig deeper with sqlite3 to see why it would miss node-gyp failing without treating it as a fatal installation error.

curtisr7 commented 8 years ago

As far as I can tell we have two problems here. The first is that sqlite3 is sort of, but not really installed. That needs to be addressed elsewhere.

The second problem is the fact that we're swallowing the meaningful error when trying to upgrade the database. I'll submit another PR to change the error message that pm writes to the console to also include the root error. Does that seem like a reasonable approach at this point?

Pete-St commented 8 years ago

Yes, that seems right to me. Thanks for the fast response!

curtisr7 commented 8 years ago

@PeteStein I merged the change... but I'm fairly sure that it's just going to tell you that strong-pm still indirectly requires sqlite3 via minkelite.... we're looking into that.