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

Update to use postgres drivers in the sqlsource package #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

llexical
Copy link

DEPENDENCY: sqlsource UPDATE
Removes the driver and instead imports the file from sqlsource to centralise all the code.

There are 3 pull requests in total for this:

  • Updating segment-sources/sqlsource to include the drivers
  • Updating segment-sources/mysql so it uses the main packages MySQL driver
  • Updating segment-sources/postgres so it uses the main packages Postgres driver

@sperand-io
Copy link
Contributor

sperand-io commented May 5, 2017

@llexical sorry for the delay here :(

i'm just sort of acclimating in these oss sql sources. do you think it makes more sense to colo all the drivers in one package (consumable as libs) or to just make them consumable from within their own repos? i'm totally open to either, just not totally sure what the convention would be /shrug

@tejasmanohar @calvinfo do either of yall have thoughts? i'd like to at least come up with a general plan for cleaning up these sql sources such that

  • each repo exposes a runnable program & container
  • someone could also use the core logic as
  • someone can add a new driver easily, following a clear convention

and... importantly

  • the drivers and domain logic is abstracted from the program/cli such that each could be consumed independently from any custom runner, which can do things like pass in a substitute publish function to target other systems or proxies (similar to how our internal etl-mongo consumes segment-sources/mongodb... or how i'd like to use the PG driver for etl-bookkeeper)

maybe we should just have a single repo (sqlsource), with all the drivers, and have a single cli that take an arg for db type that pulls in the correct driver?

@tejasmanohar
Copy link

I prefer to not include Postgres or MySQL-specific code in the sqlsource repo. I like the approach of #7. That said, I'm +1 for any shared interfaces and such in the sqlsource repo.

@llexical
Copy link
Author

llexical commented May 5, 2017

I agree that I prefer that each driver should have its own repo, personally I did not need any of the command line interface so being able to just install what I needed was much cleaner.

I like the idea of having a single cli that you can pull a db arg into, makes the cli a lot more consumable for users.

My PR is just a suggestion to get the ball rolling per say so if you come up with a better solution I am up for it, its just code I am using at the moment so I could write my own worker to run updates.

@tejasmanohar
Copy link

Cool, thanks! I think we'll go with

  • separate repo for each db source
  • use shared basedriver in sqlsource lib
  • expose db-specific driver at root of each pkg but CLI in cmd/

That way, you never need to install what you don't need

@sperand-io
Copy link
Contributor

sweet— this sounds good. i'm going to start a project over in the sqlsource repo to track the work in a central location.

@sperand-io
Copy link
Contributor

big thanks to you both @tejasmanohar @llexical!

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

Successfully merging this pull request may close these issues.

3 participants