Open Stack

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 <boris at 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.washenberger at 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 <vsergeyev at 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 at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130819/7cc15a85/attachment.html>