Skip to content

SQLite3, MySQL, Postgres Support #94

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

Closed
wants to merge 7 commits into from

Conversation

Fusion
Copy link
Collaborator

@Fusion Fusion commented Aug 31, 2019

UPDATE: Well, I added support for MySQL and Postgres. Hopefully this make needing CGO a bit more palatable!

Hi,

Apologies for the quality as I am not a Go programmer (yet) and some aspects of Go are surprising to me.
Anyway, this is a SQLite backend, with the limitations described in the header I added to the top of 'sqlitebackend.go`

I did not tag it as 'WIP' because I would like a review, but feel free to consider it as such because:
a) it's not that great
b) you may have a few suggestions for me
c) you may not agree with file naming or content, etc.

Now, this is my second attempt because I did not realize that, by requiring the SQLite library, I was creating a CGO dependency. This means that this additional feature is adding a lot of weight to the build process: Travis takes more than 10 minutes to build and you will see in the Makefile that we have to drag lots of dependencies in to build various targets. This means, too, that these targets are not directly buildable on their own platform: building for Darwin works better when running on Linux(!) -- this could be avoided by creating cross-compile targets vs native targets.
And, yes, we have to juggle the cross-compilation packages because they are not compatible, especially the 'multilib' ones which mutually uninstall; and that is why we have to add package management to the Makefile itself.

Regards

@Fusion Fusion changed the title SQLite3 Support SQLite3 Support - WIP Aug 31, 2019
@Fusion Fusion changed the title SQLite3 Support - WIP SQLite3 Support Aug 31, 2019
@Fusion Fusion changed the title SQLite3 Support SQLite3, MySQL, Postgres Support Sep 4, 2019
@Gurkengewuerz
Copy link

Gurkengewuerz commented Oct 30, 2019

I would really appreciate to see that in the official release!
Good job.

Maybe mention @benyanke 🤔

Copy link

@miolini miolini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

@butonic
Copy link
Collaborator

butonic commented Feb 21, 2020

@Fusion hey, thx for all the work. I am trying to get some PRs merged and tag a release asap. Your PR adds ~1k lines of code. While they look good I'd like to defer merging them after the next release. Also I plan to refactor the code into reusable packages: see #100 for how that will look like.

Can I ask you to split your PR into a dedicated one for every backend? Then we can get them in one by one. I admit I personally don't like the cgo dependency, we can reevaluate that after mysql and postgres are merged. That way we can see when travis starts crying ;-)

To be honest: I don't want to maintain sql backends, because it seems to me that a proper LDAP server makes more sense in those scenarios.

Now, don't let that stop you. I see the maintainer role as getting out releases on a regular base and reviewing some core PRs. For the time being core seems to be the config and ldap backends. So, I plan to release with bugs, as long as the config and ldap backends work correctly.

I am aware of the favor of other people for this. Just keep the 'lightweight' in mind.

I think we should try to gradually build a better tool. Let me know if that works for you!

This was referenced Feb 25, 2020
@Fusion
Copy link
Collaborator Author

Fusion commented Mar 5, 2020

Sure, I get your philosophy.
I'll look into the reusable package approach. Ideally these could be plug ins. Not sure how well Go supports that.

@hongkongkiwi
Copy link

How close are we to getting this into the main version? Lack of mysql support is the reason I didn't want to deploy this yet because I want to integrate everything with mysql.

@Fusion
Copy link
Collaborator Author

Fusion commented Apr 7, 2020

@hongkongkiwi I recently got around to trying to modularize the code, as previously discussed. Unfortunately, the main code has been heavily refactored since I created this pull request. I think I am now going to have to study the latest version of GLauth's code and rewrite the my SQL code. I am trying to figure out which is the least painful path, forwarding and merging wise.

@Fusion
Copy link
Collaborator Author

Fusion commented Apr 20, 2020

@butonic New pull request: #133

@hongkongkiwi Hopefully this brings you some joy :)

@Fusion Fusion closed this Apr 20, 2020
@hongkongkiwi
Copy link

hongkongkiwi commented Apr 20, 2020

I am extremely joyous! Thank you!!

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

Successfully merging this pull request may close these issues.

5 participants