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

Introduce SqlAlchemy as ORM #3

Closed
wants to merge 28 commits into from
Closed

Conversation

georg-rath
Copy link
Contributor

Hi Robert,
Hi Mark,

to make XALT able to speak with other databases than MySQL and to make life easier when working with databases, I have refactored XALT to use SqlAlchemy, which is an object-relational-mapper. It basically maps classes to database tables.
You'll find the classes in xalt_db_model.py. This model should be compatible with existing XALT databases. I wrote a small set of (very) rudimentary tests to make sure, that reading/writing to the database via the XALTdb class works.
The config file takes a connection string, instead of separate user, password, hostname and database settings. Format of the string is "[database dialect]://[username]:[password]@[host]/[database]", as documented here.
Creation of the database schema and checking for validity is done automatically by SqlAlchemy, when connecting to the database.

I think having an ORM to take care of the database handling is much more flexible, than doing all that stuff by hand. Although it creates and additional layer of abstraction I find it much easier to work with Python classes and let SqlAlchemy do the job of pushing that into the database. Using SqlAlchemy still allows you to use raw SQL to query the database.

Further work here is definitely adding more meaningful tests and introducing a tool that can handle database migrations (schema changes), like Alembic.

Let me know if you think this is worth merging.

Best regards,
Georg

/cc @itkovian @JensTimmerman @boegel

@boegel
Copy link

boegel commented Jan 9, 2015

@georg-rath: wow, impressive (although I won't pretend to have reviewed all of this)

unmergeable for now though, due to upstream changes, so you'll need to resync with upstream master?

@pforai
Copy link

pforai commented Jan 9, 2015

I think there were some commited but unpushed changes that @rtmclay just pushed a couple of minutes ago. Georg rebasing now :)

@rtmclay rtmclay closed this Apr 19, 2018
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.

None yet

5 participants