Just a related question, Oslo 'incubator' db code I think depends on eventlet. This means any code that uses the oslo.db code could/would(?) be dependent on eventlet.
Will there be some refactoring there to not require it (useful for projects that are trying to move away from eventlet). https://github.com/openstack/oslo-incubator/blob/master/openstack/common/db/sqlalchemy/session.py#L248 From: Boris Pavlovic <bo...@pavlovic.me<mailto:bo...@pavlovic.me>> Reply-To: OpenStack Development Mailing List <openstack-dev@lists.openstack.org<mailto:openstack-dev@lists.openstack.org>> Date: Monday, August 19, 2013 2:12 PM To: OpenStack Development Mailing List <openstack-dev@lists.openstack.org<mailto:openstack-dev@lists.openstack.org>> Subject: Re: [openstack-dev] [Glance] Replacing Glance DB code to Oslo DB code. Mark, But for a variety of reasons, I do not consider the general thrust of "use oslo db code" to be approved. Instead, lets continue to consider features from olso db on a case by case basis, and see what the right resolution is in each case. Absolutely agree with this point (e.g. we removed shadow tables from our roadmap after some discussion in other threads) So we are planing to make all changes using our common approach called "baby steps" (Not by one giant patch set). Btw I answered on your question about changed conf parameter in review (I mean sql_connection to database.connection). Best regards, Boris Pavlovic --- Mirantis Inc. On Mon, Aug 19, 2013 at 9:33 PM, Mark Washenberger <mark.washenber...@markwash.net<mailto:mark.washenber...@markwash.net>> wrote: Thanks for refocusing the discussion on your original questions! Also thanks for this additional summary. I consider the patches you have up for review in glance to have a general direction-level green light at this point (though I've got a question on the specifics in the ultimate review). But for a variety of reasons, I do not consider the general thrust of "use oslo db code" to be approved. Instead, lets continue to consider features from olso db on a case by case basis, and see what the right resolution is in each case. Thanks for your patience and forbearance, hopefully getting in the patches you have submitted now will help unblock progress for your team. On Mon, Aug 19, 2013 at 3:49 AM, Boris Pavlovic <bo...@pavlovic.me<mailto:bo...@pavlovic.me>> wrote: Mark, Main part of oslo is: 1) common migration testing 2) common sqla.models 3) common hacks around sqla and sqla-migrate 4) common work around engines and sessions All these points are implemented in Glance almost in the same way as in Oslo. Also we are able to use only part of this code in Glance, and add some other things that are glance related over this code. Our current 2 patches on review do next things: 1) Copy paste oslo.db code into glance 2) Use sqla session/engine/exception wrappers 3) Remove Glance code that covers session/engine/exception So I really don't see any bad thing in this code: 1) If you would like to implement other backends => this change won't block it 2) If you would like to make some other sqla utitlites or glance related things => this change won't block it 3) If there are bugs => fix it in oslo and sync => this change won't block it So I really don't see any reason to block work around migration to oslo.db code in Glance. Best regards, Boris Pavlovic --- Mirantis Inc. On Fri, Aug 16, 2013 at 10:41 PM, Mark Washenberger <mark.washenber...@markwash.net<mailto:mark.washenber...@markwash.net>> wrote: I would prefer to pick and choose which parts of oslo common db code to reuse in glance. Most parts there look great and very useful. However, some parts seem like they would conflict with several goals we have. 1) To improve code sanity, we need to break away from the idea of having one giant db api interface 2) We need to improve our position with respect to new, non SQL drivers - mostly, we need to focus first on removing business logic (especially authz) from database driver code - we also need to break away from the strict functional interface, because it limits our ability to express query filters and tends to lump all filter handling for a given function into a single code block (which ends up being defect-rich and confusing as hell to reimplement) 3) It is unfortunate, but I must admit that Glance's code in general is pretty heavily coupled to the database code and in particular the schema. Basically the only tool we have to manage that problem until we can fix it is to try to be as careful as possible about how we change the db code and schema. By importing another project, we lose some of that control. Also, even with the copy-paste model for oslo incubator, code in oslo does have some of its own reasons to change, so we could potentially end up in a conflict where glance db migrations (which are operationally costly) have to happen for reasons that don't really matter to glance. So rather than framing this as "glance needs to use oslo common db code", I would appreciate framing it as "glance database code should have features X, Y, and Z, some of which it can get by using oslo code." Indeed, I believe in IRC we discussed the idea of writing up a wiki listing these feature improvements, which would allow a finer granularity for evaluation. I really prefer that format because it feels more like planning and less like debate :-) I have a few responses inline below. On Fri, Aug 16, 2013 at 6:31 AM, Victor Sergeyev <vserge...@mirantis.com<mailto:vserge...@mirantis.com>> wrote: Hello All. Glance cores (Mark Washenberger, Flavio Percoco, Iccha Sethi) have some questions about Oslo DB code, and why is it so important to use it instead of custom implementation and so on. As there were a lot of questions it was really hard to answer on all this questions in IRC. So we decided that mailing list is better place for such things. List of main questions: 1. What includes oslo DB code? 2. Why is it safe to replace custom implementation by Oslo DB code? 3. Why oslo DB code is better than custom implementation? 4. Why oslo DB code won’t slow up project development progress? 5. What we are going actually to do in Glance? 6. What is the current status? Answers: 1. What includes oslo DB code? Currently Oslo code improves different aspects around DB: -- Work with SQLAlchemy models, engine and session -- Lot of tools for work with SQLAlchemy -- Work with unique keys -- Base test case for work with database -- Test migrations against different backends -- Sync DB Models with actual schemas in DB (add test that they are equivalent) 2. Why is it safe to replace custom implementation by Oslo DB code? Oslo module, as base openstack module, takes care about code quality. Usually, common code more readable (most of flake8 checks enabled in Oslo) and have better test coverage. Also it was tested in different use-cases (in production also) in an other projects so bugs in Oslo code were already fixed. So we can be sure, that we use high-quality code. Alas, while testing and static style analysis are important, they are not the only relevant aspects of code quality. Architectural choices are also relevant. The best reusable code places few requirements on the code that reuses it architecturally--in some cases it may make sense to refactor oslo db code so that glance can reuse the correct parts. 3. Why oslo DB code is better than custom implementation? There are some arguments pro Oslo database code -- common code collects useful features from different projects Different utils, for work with database, common test class, module for database migration, and other features are already in Oslo db code. Patch on automatic retry db.api query if db connection lost on review at the moment. If we use Oslo db code we should not care, how to port these (and others - in the future) features to Glance - it will came to all projects automaticly when it will came to Oslo. -- unified project work with database As it was already said, It can help developers work with database in a same way in different projects. It’s useful if developer work with db in a few projects - he use same base things and got no surprises from them. I'm not very motivated by this argument. I rarely find novelty that challenging to understand when working with a project, personally. Usually I'm much more stumped when code is heavily coupled to other modules or too many responsibilities are lumped together in one module. In general, I'd take new ravioli code over familiar spaghetti code any day. -- it’s will reduce time for running tests. Maybe it’s minor feature, but it’s also can be important. We can removed some tests for base `DB` classes (such as session, engines, etc) and replaced for work with DB to mock calls. 4. Why oslo DB code won’t slow up project development progress? Oslo code for work with database already in such projects as Nova, Neutron, Celiometer and Ironic. AFAIK, these projects development speed doesn’t decelerated (please fix me, If I’m wrong). Work with database level already improved and tested in Oslo project, so we can concentrate on work with project features. All features, that already came to oslo code will be available in Glance, but if you want to add some specific feature to project *just now* you will be able to do it in project code. I think the issue here for glance is whether or not oslo common code makes it easier or harder to make other planned improvements. In particular, using openstack.common.db.api will make it harder to refactor away from a giant procedural interface for the database driver. 5. What we are going actually to do in Glance? -- Improve test coverage of DB API layer We are going to increase test coverage of glance/db/sqlalchemy/api module and fix bugs, if found. -- Run DB API tests on all backends -- Use Oslo migrations base test case for test migrations against different backends There are lot of different things in SQl backends. For example work with casting. In current SQLite we are able to store everything in column (with any type). Mysql will try to convert value to required type, and postgresql will raise IntegrityError. If we will improve this feature, we will be sure, that all Glance DB migrations will run correctly on all backends. -- Use Oslo code for SA models, engine and session -- Use Oslo SA utils Using common code for work with database was already discussed and approved for all projects. So we are going to implement common code for work with database instead of Glance implementation. I'm not sure what you mean by "discussed and approved for all projects". Isn't that discussion basically happening now for glance? -- Fix work with session and transactions Our work items in Glance: - don't pass session instances to public DB methods - use explicit transactions only when necessary - fix incorrect usage of sessions throughout the DB-related code -- Optimize methods When we will have tests for all functions in glance/db/sqlalchemy/api module it’s will be safe to refactor api methods. It will make these functions more clean, readable and faster. The main ideas are: - identify and remove unused methods - consolidate duplicate methods when possible - ensure SQLAlchemy objects are not leaking out of the API - ensure related methods are grouped together and named consistently -- Add missing unique constraints We should provide missed unique constraints, based on database queries from glance.db.sqlalchemy.api module. It’s will reduce data duplication and became one more step to Glance database normalization. -- Sync models definitions with DB migrate scripts At the moment we do not use Glance db models for db creation. To create db we use migrations. We don't have any tests that checks, that our models are up-to-date. Also we are testing it only on sqlite, which couldn't test such things as nullable constraints. So we plain fix all mistakes in models and migration, sync effects of migrations in different backends and add tests that ensures that models are up-to-date. -- Use alembic for database migrations SQLAlchemy-migrate (Glance database migration tool) is fine for straight-line migrations, but does not really support branching, needed for clean backports of some but not all migrations to a stable branch. Also this project doesn't seems to be live. Alembic has better branching support and will meet our needs better. We should switch migration frameworks, and convert our existing migration scripts to Alembic syntax. -- DB Reconnect There are a variety of circumstances which can cause a transient failure in database connections, for example: restart / upgrade of the database, just a network failure, etc. Glance (as all projects connecting to a database) would benefit from the db/api catching these "db-has-gone-away" errors and automatically reconnecting and retrying the last db api method call. It is not necessary to abort long-running operations (such as glance image-create) just because of a momentary interruption in db connectivity. -- No downtime database upgrade No comments 6. What is the current status? -- Improve test coverage of DB API layer Not started -- Run DB API tests on all backends Not started -- Use Oslo code for SA models, engine and session Started -- Use Oslo migrations base test case for test migrations against different backends -- Use Oslo SA utils In progress at the moment. During the implementation, such changes was made: - DB layer code cleanup. We removed project-specific functions for work with DB layer in db.api. Now we use common Oslo code. - Base class for DB models from common code was used, instead of project base model class. - Refactored migration module. Also we plan to use common module for migrations from Oslo - Race condition in migration 012 was fixed. - Tests, based on module glance.tests.integration.legacy_functional.base use sqlite in memory now. - Modified work with config - we use some config values from Oslo code. - Removed some tests for base class for work with DB - functions for work with DB was already tested in Oslo project. - Renamed unique constraints due to common name convention. - Patched sqlalchemy-migrate to fix UC bugs in SQLite. -- Fix work with session and transactions Not started -- Optimize methods Not started -- Add missing unique constraints Not started -- Sync models definitions with DB migrate scripts Mostly done. -- Use alembic for database migrations Not started -- DB Reconnect On review in Oslo. We will be able to use this feature after we will fix work with session and transactions Thanks, Victor. _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org<mailto:OpenStack-dev@lists.openstack.org> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org<mailto:OpenStack-dev@lists.openstack.org> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org<mailto:OpenStack-dev@lists.openstack.org> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org<mailto:OpenStack-dev@lists.openstack.org> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
