-
Notifications
You must be signed in to change notification settings - Fork 3
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
Comments
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. |
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. |
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?
Makes sense. @argvk suggested the exact same thing when we had a chat offline. Hehe. |
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? |
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. |
Agree. Allright I'll put it on the roadmap then :) Thanks for the inputs! I'll leave this issue open for further discussion |
Was thinking about taking a crack at this this weekend if nobody is currently working on it. |
Hii. No one currently is, Please feel free to take a crack at it 👍 😄 |
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. |
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 |
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. |
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.
The text was updated successfully, but these errors were encountered: