Access to SQL console about hanami HOT 17 CLOSED

@joneslee85 I'd like to give this a shot if that is OK :)

I have some questions though since I'm quite new to Lotus.

1. It is possible to configure adapters in both lib/<app_name>.rb as well as apps/<app>/application.rb as far as I can tell. What is the difference and how do we know where to look?
2. I use Lotus::Model so for me it would work to require lib/<app_name>.rb and then check Lotus::Model.configuration.adapter but then we would be dependent on Lotus::Model which is not good.
   Is there a generic way to get the adapter configuration?

from hanami.

@Linuus @joneslee85 I'd say to start simple with this and support only the database used in lib/.

The data that you need is the type of database and a connection string. So I was thinking if it may be worth to read it from the dotenv files, without introspecting Lotus::Model.

When you create an application with lotus new bookshelf, the CLI creates some files under config/.
The database configuration is named after the application (eg. BOOKSHELF_DATABASE_URL).

What if we generate it only with DATABASE_URL so this command just need to lookup that env var and start a database REPL?

Thoughts?

from hanami.

@jodosha Imagine you have two lotus applications in the same machine, an they are using the same DATABASE_URL, will it be a problem with this?

from hanami.

@AlfonsoUceda good point. 👍

I suggest to generate a .lotusrc file with those preferences:

application_name: bookshelf

So it will be able to compose the dynamic part from that configuration and concat with ENV["#{ application_name.upcase }_DATABASE_URL"].

This can be useful for code generators too.

from hanami.

👍

from hanami.

The only downside of bypassing the adapter is to maintain here the logic that passes the right args to the different database CLIs.

Probably the adapter should indicate the right string to start the REPL.

from hanami.

@jodosha So when generating a new Lotus app we should create a .lotusrc file and put the app name in there in yml format? Then read this in the dbconsole command and find the correct env variable.

Why not pass in the app name to the command?
lotus dbconsole bookshelf <-- Connects to BOOKSHELF_DATABASE_URL
lotus dbconsole web <-- Connects to the WEB_DATABASE_URL

from hanami.

@Linuus That could be an alternative. But again, I'm thinking if it's better to let the adapters to return the CLI connection string.

adapter.connection_string # => "psql -h localhost" etc.
adapter.connection_string # => "mysql -h localhost" etc.

It this way an adapter author will implement that method and we can hook lotus dbconsole with that output.

adapter.connection_string # => "redis ..."

If we don't apply this strategy, we're forced to maintain all those connections strings here in lotusrb, which doesn't seems to be the best way to go.

I suggest to do:

lotus dbconsole # loads main adapter from lib/ (Lotus::Model.configuration.adapter)
lotus dbconsole web # Web::Model.configuration.adapter

from hanami.

@jodosha Yes I agree with that. I looked at Rails console command and it is messy because they do all that stuff in there.

 I'll give this a go during the weekend! :)

On Fri, Jan 16, 2015 at 7:50 PM, Luca Guidi [email protected]
wrote:

@Linuus That could be an alternative. But again, I'm thinking if it's better to let the adapters to return the CLI connection string.

adapter.connection_string # => "psql -h localhost" etc.
adapter.connection_string # => "mysql -h localhost" etc.

It this way an adapter author will implement that method and we can hook lotus dbconsole with that output.

adapter.connection_string # => "redis ..."

If we don't apply this strategy, we're forced to maintain all those connections strings here in lotusrb, which doesn't seems to be the best way to go.
I suggest to do:

lotus dbconsole # loads main adapter from lib/ (Lotus::Model.configuration.adapter)
lotus dbconsole web # Web::Model.configuration.adapter

---

Reply to this email directly or view it on GitHub:
#123 (comment)

from hanami.

I'm only concerned about security: this command needs to trust that the returned REPL command isn't evil, in case if third party adapters.

http://lucaguidi.com

On 16/gen/2015, at 19:55, Linus Pettersson [email protected] wrote:

@jodosha Yes I agree with that. I looked at Rails console command and it is messy because they do all that stuff in there.

I'll give this a go during the weekend! :)

On Fri, Jan 16, 2015 at 7:50 PM, Luca Guidi [email protected]
wrote:

@Linuus That could be an alternative. But again, I'm thinking if it's better to let the adapters to return the CLI connection string.

adapter.connection_string # => "psql -h localhost" etc. 
adapter.connection_string # => "mysql -h localhost" etc. 

It this way an adapter author will implement that method and we can hook lotus dbconsole with that output.

adapter.connection_string # => "redis ..." 

If we don't apply this strategy, we're forced to maintain all those connections strings here in lotusrb, which doesn't seems to be the best way to go.
I suggest to do:

lotus dbconsole # loads main adapter from lib/ (Lotus::Model.configuration.adapter)
lotus dbconsole web # Web::Model.configuration.adapter 

---

Reply to this email directly or view it on GitHub:
#123 (comment)
—
Reply to this email directly or view it on GitHub.

from hanami.

@Linuus thanks for your offer, however @unrealhoang has been working on this feature as per #126. Can you please chat to @unrealhoang on chat room to see if he is happy to let you take over?

@jodosha I am feeling uneasy with the approach reading from dotenv ENV vars, firstly there is no hard convention on the naming of these var, user can change the naming of ENV. Secondly, it is hard to tell from the dotenv var if the adapter is truly SQL or not and which right params should be parsed into the CLI.

Thus, I propose that we should stick to the approach of reading from adapter configuration like I proposed before.

from hanami.

@jodosha @joneslee85 Where did we land regarding letting the adapter specify the command to run the actual console?

There are two options as discussed above.

1. Add the logic of figuring out the adapter type and command to run in the dbconsole command. (Safer, but we need to maintain it).
2. Add a new interface to the adapter where the adapter developer adds a method returning the command to run and we just execute it. (Less safe, easier to maintain).

Option 2 seems more logical to me. Any ideas on how to avoid the security issues?

from hanami.

To have something to discuss around I have made a quick take on this. This requires changes to Lotus::Model as well to add the adapter.connection_string @jodosha was talking about above. It is not a complete PR yet but here are the relevant commits.

Changes to Lotus::Model:
Linuus/model@cff0027

Changes to Lotus:
Linuus@b0cf570

Any thoughts? Should I clean it up and make a PR? Should we take another approach all together?

from hanami.

@Linuus looks good. I like it. Please clean up and make a PR. Much thanks

from hanami.

@joneslee85 Great, thanks for the feedback!

from hanami.

@Linuus wondering if you need any help on this?

from hanami.

@joneslee85 Not at the moment. I have just had a lot to do at work lately. I am not really sure how to test the CLI part but I'll have a look at the other tests and see if I can figure it out. Otherwise I'll send you a message :)

from hanami.