| stephenfin | zzzeek: How on earth did you find that? Argh, that's so irritating! ðŸ˜ðŸ˜ðŸ˜… | 09:26 |
|---|---|---|
| *** dviroel_ is now known as dviroel | 12:05 | |
| opendevreview | Stephen Finucane proposed openstack/keystone master: sql: Integrate alembic https://review.opendev.org/c/openstack/keystone/+/825844 | 12:30 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: docs: Update docs to reflect migration to Alembic https://review.opendev.org/c/openstack/keystone/+/828905 | 12:30 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: WIP: sql: Add support for auto-generation https://review.opendev.org/c/openstack/keystone/+/826147 | 12:30 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: tests: Don't monkeypatch functions https://review.opendev.org/c/openstack/keystone/+/846612 | 12:30 |
| zzzeek | stephenfin: testr makes it hard to select for tests so i started by making a subdirectory "migrations" and putting test_* files in there along with the two alembic-oriented ones that kept having sporadic failures | 14:13 |
| zzzeek | stephenfin: that way I could run testr and get failures very fast within a few seconds. then i set out to just bisect the rest of the files in the owning directory to see which one triggered it, and test_cmd looked like a prime candiate since it's running the commands, which I assumed included the migration commands | 14:14 |
| zzzeek | stephenfin: this went very quickly because your implementation works perfectly great | 14:14 |
| zzzeek | stephenfin: my fear was that something was subtly wrong with the integration, or it would be one of those cases where failure happens if test_X and test_Y are both present, but not one or the other | 14:15 |
| zzzeek | but it was just that one dumb thing, so kudos that the implementation appears to work great | 14:15 |
| stephenfin | zzzeek++ Nicely done. I was certain it was a shared global issue and got fixated on that. Never even crossed my mind it could be something different. Thanks very much for the assistance there :) | 15:24 |
| stephenfin | zzzeek: Hopefully one final question (low priority): I see this bit of logic in the neutron code https://github.com/openstack/neutron/blob/master/neutron/db/migration/autogen.py#L102-L109 | 15:29 |
| zzzeek | divide and conquer is a tool I reach for quickly | 15:29 |
| zzzeek | stephenfin: so i immediately clicked on blame to say, no idea what that is, lets see who wrote it | 15:30 |
| zzzeek | big mistake :) | 15:30 |
| stephenfin | 7 years is a long time :) | 15:30 |
| zzzeek | stephenfin: im amazed i have absolutely zero recollection of writing code like this | 15:32 |
| stephenfin | Just to confirm, that's saying that only modify nullable alter statements should be allowed during the "expand" phase, right? | 15:33 |
| zzzeek | it appears so. it looks like this code is classifying migration operations as expansive or contract automatically | 15:34 |
| stephenfin | that's my thinkin | 15:34 |
| stephenfin | *g | 15:34 |
| stephenfin | It's all black magic to me, though I'm slowly getting to grips with it thankfully | 15:34 |
| stephenfin | Would a doc/example code on this expand/contract design be useful in the alembic docs? I wasn't able to find anything on the idea other than this neutron code that you wrote :) | 15:35 |
| stephenfin | it's probably not important enough to be alembic core code or it would already be there | 15:35 |
| zzzeek | stephenfin: i would liekly read the blueprint that this code seems to come from | 15:36 |
| zzzeek | i probably wrote it | 15:36 |
| zzzeek | blueprint online-schema-migrations for neutron | 15:36 |
| *** dviroel is now known as dviroel|lunch | 15:36 | |
| zzzeek | i owuld read it too if you know how to find the link | 15:36 |
| stephenfin | Already in my history from a few months back https://specs.openstack.org/openstack/neutron-specs/specs/liberty/online-schema-migrations.html | 15:37 |
| stephenfin | However, it mostly focuses on the "why" rather than the "how" (as a good spec should tbf) | 15:37 |
| zzzeek | stephenfin: the general thing this patch is doing is using an alembic hook called process_revision_directives, which is called only when one calls alembic revision --autogenerate | 16:21 |
| zzzeek | stephenfin: it apepars that it will receive autogenerated migrations and classify them into expand and contract and separate them accordingly | 16:21 |
| zzzeek | there's not a recipe in the docs for this particular use case right now, I've ceratinly offered to other users they might want to try expand/contract | 16:23 |
| stephenfin | That's what I've roughly figured out. That bit of logic I linked to earlier is still weird though. Why can't we determine whether a modification to the nullable value is an expand or contract thing? | 16:23 |
| stephenfin | Surely setting something to nullable=True makes the schema more permissive, which means it's an expand, and vice versa? | 16:24 |
| stephenfin | Context is that auto-generation is currently broken for keystone because there are a couple of discrepancies between the schemas and migrations that still need to be cleaned up. You can see me filtering them out in the 'include_object' and 'filter_metadata_diff' methods here | 16:26 |
| stephenfin | https://review.opendev.org/c/openstack/keystone/+/825844/8/keystone/tests/unit/common/sql/test_upgrades.py#101 | 16:26 |
| stephenfin | some of which are changes to nullable attributes which bork auto-generation. I can skip these but I'd like to know if it's needed first :) | 16:26 |
| *** dviroel|lunch is now known as dviroel | 16:43 | |
| zzzeek | stephenfin: if i have a nullable column and im setting it to not-null, yes i guess that is a contract and i think that's waht this code is doing already | 17:39 |
| zzzeek | stephenfin: modify_nullable = (False|True) means, "change the nullability" | 17:39 |
| zzzeek | stephenfin: modify_nullable=None means, "dont cahnge the nullability" | 17:39 |
| zzzeek | stephenfin: so that notimpl means, there's an alter column that has no change in nullability and it cant be detected if the alter column is an expand or a contract as a result | 17:40 |
| mloza1 | https://docs.openstack.org/api-ref/identity/v3/?expanded=create-application-credential-detail#id113 - this allows me to create app cred in behalf of the user | 17:59 |
| mloza1 | just wanted to confirm | 17:59 |
| *** dviroel is now known as dviroel|afk | 20:05 | |
| *** dviroel|afk is now known as dviroel|out | 21:05 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!