Russell Keith-Magee wrote: > Documentation should be about how to use a feature, not how the > internals of a feature work.
fixed in the branch. plus it now includes installation instructions, a future work section, etc. also available here: http://code.djangoproject.com/wiki/SchemaEvolutionDocumentation > However, reading between the lines, here are a few quick comments: > > - The introspection stuff is neat, but I _really_ don't like the aka > representations in the model. The models file should always be a clean > statement of the current state of the model. Migration is about > getting an old database to match the currently required models - if I > don't have a legacy database, I don't really want the cruft hanging > around in my models. Migration plans or historical models really > should be kept out of the core model file, IMHO. We currently store all sorts of non-DB related metadata in the model that arguably should not be there, including presentation information. We do this for clarity and convenience - you would have to duplicate a lot of information otherwise in multiple locations without any obvious direct connection. So which paradigm is more critical here, DRY or MVC? Or do we continue with the status-quo of finding a common-sense balance? As far as cruft, if you don't have a legacy database, you wouldn't have any aka fields to begin with. And as you phase out legacy support, simply delete them. > - Unless I'm missing something, the aka approach doesn't track exactly > which name versions correlate with other versions. Consider; your aka > chain for one field could indicate a rename from 'foo' to 'bar' to > 'foo' to 'bar'; a second field could indicate renames from 'whiz' to > 'foo' to 'whiz', etc. A tuple of historical names doesn't tell me > which sets of names were in use concurrently; so if I find a database > with a field called 'foo' which requires migration, is 'foo' the first > field or the second? Correct, however I thought this to be a highly unlikely scenario, not warranting the extra notational complexity. But just as we support strings and tuples, there is nothing to say we can extend it to support say, a mapping of historical names to date ranges or version numbers, if the need arises. > - Your patch doesn't seem to address issue of cross-platform SQL. [...] > I can't see anything in your code that provides an alternate mechanism > for removing columns, or providing a warning if you try to remove a > column from an SQLite table. db-specific issues are handled in the backend, behind the api interface used by all the backends. for your example, deleting a column in sqlite generates the following warning: -- FYI: sqlite does not support deleting columns, so 'col_name' remains as cruft > - Your patch doesn't seem to address the issue of sequential evolution > - changes that must be applied sequentially (rather than just > adding/removing columns to get you from current database state to > current model definition). This is particularly relevant when you > start looking at doing data conversion as part of the migration > process we do handle at least some sequential changes. for example: renaming a column while you're renaming the table it's in. and in the future work section i talk about how data conversion would work, but that wouldn't require any user-visible changes and doesn't impede the current functionality. give me some scenarios for where you're concerned. > I don't see how you can test schema-evolution without reloading > models. The tests you have are useful - they demonstrate that the > right SQL is generated - but this doesn't help if the SQL can change > on a per-backend basis. i agree, this is needed. if anyone knows how to add a field to an already-loaded class in python, please send me a link to the process. i could find adding methods, but not fields. also, yes, currently only the MYSQL backend is tested. postgres and sqlite tests are coming... (and yes, the sql does change on a per-backend basis) > SQL generation is also the least complex part of the entire process. > Your tests don't check that complex part - the introspection is being > done correctly. There is no use in generating the correct SQL for a > rename if you can't identify what constitutes a rename :-) look closer - the introspection is being called and tested in each of the examples. (i'm not just calling the sql-generation methods) but i do admit that the tests are not comprehensive. i figure release early/often. :) > Yours, > Russ Magee %-) glad for the constructive criticism. :) i think the only thing we're too far apart on is the notation. i'm not wed to it, but i do think light-weight/in-the-model is the way to go, esp. considering how little metadata we're storing. i'll send notice when i get the postgres/sqlite scripts commited. -- derek --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django users" group. To post to this group, send email to django-users@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-users?hl=en -~----------~----~----~----~------~----~------~--~---
