mtxr / SublimeText-SQLTools

SQLTools for Sublime Text 3
https://code.mteixeira.dev/SublimeText-SQLTools/
GNU General Public License v3.0
177 stars 40 forks source link

Dynamic connections #100

Closed josecanciani closed 7 years ago

josecanciani commented 7 years ago

Issue Type

Description

Hi! I found your plugin super useful. I did a very similar implementation, although with a lot less features. In our team we work with a lot of cloud databases, so we need to forward an ssh port for connection. I'm interested of using SublimeTools for working directly with them, but in order to do that, I need some features. I'm putting all of them here, it probably makes more sense to have different Issues for each, but I want your opinion first. I wrote them in order of importance for us.

1) Support connections per file. By having a comment like:

-- ST:mydb

at the beginning of the file, the plugin can use the "mydb" connection instead of having to force the user to manually switch between connections.

2) Be able to run something before starting a connection. I need to run an ssh with local port forwarding for enabling the connection. The parameter for the script should be dynamic, something like:

{
    "pre_connect_command": "/home/jose/bin/openMysqlTunnel.sh {$DBNAME}"
}

3) Dynamic database configuration: instead of hard-coding connections into a json file, have a way to get connections details dynamically from a different place (we have almost 1000 dbs, each with a replica). I'm thinking on something like the Hook pattern (not sure if Sublime has some kind of inter-plugin communication support). It could also implementing by running a script that will spit some json connections.

{
    "get_config_command": "/home/jose/bin/getDbConfig.sh {$DBNAME}"
}

4) Finally, automatic logging. If the file has been saved (let's say I'm working on mydb.sql) then, if a certain config is set, the plugin would append every command result to a file called mysql.sql.log.

If I can make some time to develop some of them, I will, but I want to have your opinions first to know if this is something you would like having in your plugin or not. After all, it will be you who will end up supporting this later. If you think all this is way our of your scope, I can create my own branch to work with.

Thanks! Jose

tkopets commented 7 years ago

Hello, Jose!

Thank you for your feedback and feature requests. I understand your desire to create a perfect workflow with this plugin but in my opinion, the features you request are specific only to your workflow and would be very rarely (or never) used by others.

Also from the high-level description of your workflow, I would recommend looking into utilizing Sublime Text Projects, Build systems or combination of both to solve some of the problems you outlined with more stable and widely used Sublime toolset. First of all, SQLTools should support project-specific Connection files (it is probably not listed as a feature). Also, there is a concept of default connection which would be selected automatically after SQLTools is loaded. In my opinion, project-specific connection files are a more elegant way to manage connections than listing connection in the file as a comment (the same file can be referenced from multiple projects and have different default connection).

Given that some of the options might be:

In my opinion, almost all of the features you have listed are pretty unique only for your specific workflow - having all of that functionality would unnecessarily complicate plugin code and would eventually become a maintenance burden.

This is solely my personal opinion and repo owner @mtxr might think otherwise.

josecanciani commented 7 years ago

Yeah, I understand your reasoning and I agree very much, I wouldn't want to add things that are not the main reason of my module.

Having said that, I think I will not be able to adapt to my workflow if SQLTools does not support multiple active connections defined per active file. The rest of the things I think I can easily manage to modify in a branch of my own, but having multiple connections is a must have.

I'll wait for @mtxr to shoot some comments.

Thanks!

mtxr commented 7 years ago

I completely agree with @tkopets. I would use project-settings for now.

Is it ok?

Even though, you can switch connections any time you want.

In my opinion Sublime Text should have a "Panel Like" feature like VSCode, so you could open panel tabs for each connection, that would be awesome.

josecanciani commented 7 years ago

I think I can live with per project connections. Is this something that is working now? I would expect that running ctrl E E on one project to automatically use the default, then going to a different window (another project) and without manually changing the connection, ctrl E E would run in a different one.

Regarding the panels, you can use sublime's window.find_output_panel, which finds panel by name. I did this modification to SQLTools.py in order to append to a panel instead of opening a new one everytime a new query is run:

141a142
>     isNew = True
143c144,148
<         resultContainer = Window().create_output_panel(name)
---
>         resultContainer = Window().find_output_panel(name)
>         if resultContainer:
>             isNew = False
>         else:
>             resultContainer = Window().create_output_panel(name)
149a155
>                 isNew = False
157,159c163,166
<     resultContainer.set_scratch(True)  # avoids prompting to save
<     resultContainer.settings().set("word_wrap", "false")
<     resultContainer.set_read_only(False)
---
>     if isNew:
>         resultContainer.set_scratch(True)  # avoids prompting to save
>         resultContainer.settings().set("word_wrap", "false")
>         resultContainer.set_read_only(False)
505,506c512
<             return ST.conn.execute(history.get(index), createOutput(),
<                                    stream=settings.get('use_streams', False))
---
>             return ST.conn.execute(history.get(index), createOutput())
542,549c548,550
<             alias, query = options[index]
<             if mode == "run":
<                 ST.conn.execute(query, createOutput(),
<                                 stream=settings.get('use_streams', False))
<             else:
<                 toNewTab(query, alias)
< 
<             return
---
>             param2 = createOutput() if mode == "run" else options[index][0]
>             func = ST.conn.execute if mode == "run" else toNewTab
>             return func(options[index][1], param2)

Jose

tkopets commented 7 years ago

There is no concept of multiple active connections in SQLTools. Even if you have multiple windows open in Sublime Text the plugin instance is shared between those multiple windows. Therefore the last connection that was used (or selected as default) will be active. Note: default connection is only loaded on plugin (Sublime) start. If you start Sublime and it happens to be in a project - project specific list of connections should be loaded, if default connection in supplied it will be activated during this startup process. If you open a new project or change project in Sublime that most probably will not trigger the loading of project-specific connections (but probably should).

On appending results: There is already a setting in SQLTools.sublime-settings for the functionality you referenced in your previous comment called clear_output. If that setting does not work for you - submit a separate issue or a pull request. Note: you have to restart Sublime Text to for plugin to pick up new plugin settings (probably, this can be improved).

tkopets commented 7 years ago

@mtxr As it is now selecting an active connection is pretty straightforward and simple. What do you think about internally maintaining a connection per project/window? Do you think this should be done at all? Do we have to rewrite a lot to achieve that? Even if you think this is a good idea, I wouldn't rush implementing that and rather create a separate issue and try to gather some feedback to see if it worth implementing it at all - as this will definitely complicate codebase.

tkopets commented 7 years ago

Indeed there was a bug in clear_output functionality and the panel output was always cleared (we always created an new panel) - thanks for spotting that. I've created a separate issue #102 for and fix #103 for that. The fix will be available in next release.

josecanciani commented 7 years ago

Thank you guys, I'll close the issue, but I still want a response regarding @tkopets question to @mtxr

As it is now selecting an active connection is pretty straightforward and simple. What do you think about internally maintaining a connection per project/window? Do you think this should be done at all? Do we have to rewrite a lot to achieve that?

I'll fork the project if you don't want to support it, which would probably be a lot of work to keep it synchronized. :(

It seems it should be quite straightforward: today the connection is set in ST.conn var, which could be converted into an array of connections indexed by the Window().id() -although I'll love to have also per view support, so if you consider this, use 'window-' + Window().id() so ids won't crash in the future if you want to add more connection types-.

We could have panels indexed in the same way, as well as showing the active connection in the status bar, similar to how GitGutter shows the current branch for example:

            # add text and try to be the left most one
            self.git_handler.view.set_status('00_git_gutter', text)

Please let me know what you decide!

Thanks! Jose