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

Executing SQL #4

Open
mikeshultz opened this issue Apr 6, 2015 · 11 comments
Open

Executing SQL #4

mikeshultz opened this issue Apr 6, 2015 · 11 comments

Comments

@mikeshultz
Copy link
Contributor

I'd like to request that this come with some form of database agnostic way to actually execute the queries. It could return results in the form of a CSV, or even HTML table may be useful.

I may fork this and try it out myself at some point if I have the time.

@shrayasr
Copy link
Member

shrayasr commented Apr 6, 2015

I'm keen on doing this too but I see a lot more things would have to be developed around this if we were to take this up.

Of the top of my head, It would become absolutely essential for us to introduce users and access control into the equation. Not everyone should be allowed to execute a query, only those that have rights to.

@mikeshultz
Copy link
Contributor Author

I was thinking if LSQ was connected as a read-only user, but I'm sure plenty of other folks would want full SQL users(your example had a COPY statement, after all).

Might be able to simplify it greatly if you don't bother with much of an auth system and use the database for authentication. LSQ could be entirely agnostic and just display the error messages produced by the backend.

@shrayasr
Copy link
Member

shrayasr commented Apr 6, 2015

I was thinking if LSQ was connected as a read-only user, but I'm sure plenty of other folks would want full SQL users(your example had a COPY statement, after all).

Lets work on LSQ being able to run READ ONLY queries via the UI. Any and all WRITE based operations will not be supported by LSQ. Sound good?

Might be able to simplify it greatly if you don't bother with much of an auth system and use the database for authentication. LSQ could be entirely agnostic and just display the error messages produced by the backend.

Makes sense. @argvk suggested the exact same thing when we had a chat offline. Hehe.

@shrayasr
Copy link
Member

shrayasr commented Apr 8, 2015

Lets work on LSQ being able to run READ ONLY queries via the UI. Any and all WRITE based operations will not be supported by LSQ.

On second thought, Like you said it makes sense that all of this be offloaded to the underlying DB. If it is set up with a user that has both R/W access then so be it. We'll just provide a way to execute the queries.

A follow up question to this would be if we are going to allow LSQ to be able to connect to multiple databases?

@mikeshultz
Copy link
Contributor Author

Considering the effort to set it up for one is almost exactly the same as setting it up for multiple, I don't see why not.

@shrayasr
Copy link
Member

shrayasr commented Apr 9, 2015

Agree. Allright I'll put it on the roadmap then :) Thanks for the inputs!

I'll leave this issue open for further discussion

@mikeshultz
Copy link
Contributor Author

Was thinking about taking a crack at this this weekend if nobody is currently working on it.

@shrayasr
Copy link
Member

shrayasr commented Jul 3, 2015

Hii. No one currently is, Please feel free to take a crack at it 👍 😄

@mikeshultz
Copy link
Contributor Author

Okay, so I've got a ton of stuff in my branch: https://github.com/mikeshultz/LSQ/tree/run-queries

Documentation is not updated. I have not bothered with tests, but it doesn't look like it's used, so I'm not too worried about that.

Mostly, I'd like to get some thoughts on it, and some of the design decisions I made. I did my best to stick to the current style, so hopefully that isn't an issue. However, I chose to store authentication in mongo along with the database record. It's encrypted, but currently it would only allow for one set of credentials per database.

For my purposes this would work, for others, it may not. It might be prudent to start here and make incremental improvements later(like a user/pass prompt on the run page).

It has been tested with PostgreSQL, and should work with any DB-API2 compatible engine. However, there are parts, like exception handling that may not work across different database engines because they might be outside of the DB-API2 standard.

So if you, or anyone else, wants to look it over and maybe test it with a different DB-API2 backend, that would help a lot. If it would make it easier, I could make it a pull request but I'd like to get things like documentation finished before anything gets merged.

@shrayasr
Copy link
Member

shrayasr commented Jul 4, 2015

This looks great 👍. Thanks a ton. Let me take some time out later in the day to review this code and get back. Just looked through the code and it looks great from first glance.

I'll report back with nitpicks. Thanks so much for this. Appreciate it

@mikeshultz
Copy link
Contributor Author

mikeshultz commented Mar 29, 2017

I've added PR #15. I've rebased on your master branch, included some bug fixes, and improved error handling for the query runner. Let me know if you have any feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants