| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-06 | |||
| 21:15:15 | dansmith | because I really need her to start taking some of the blame | |
| 21:15:50 | melwitt | it could be reasoned as my fault for not pushing my rreplace() agenda | |
| 21:16:10 | mriedem | it's my fault | |
| 21:16:36 | mriedem | at least we have the unit test to recreate it, just add mysql+pymysql://nova:password@db.example.com/nova?charset=utf8 to the ddt list | |
| 21:16:40 | melwitt | I'm still not sure that's really the right thing to do either because we want to make a change to the url.database variable really. it just didn't work right because of encoding or something | |
| 21:17:51 | mriedem | cool that's also in ocata and newton b/c i backported it | |
| 21:18:01 | melwitt | yeah. fun times | |
| 21:18:22 | bauzas | I just feel we need to replace at the index where we found the string | |
| 21:18:50 | mriedem | i was doing sometihng like that in an earlier version | |
| 21:18:58 | mriedem | and then the sqla url thing was pointed out to simplify | |
| 21:19:18 | melwitt | bauzas: yeah, my suggestion in the review was "return cell0_db_name.join(connection.rsplit(url.database, 1))" | |
| 21:19:32 | melwitt | but really we want to just change url.database and then re-make the whole url and return it | |
| 21:19:46 | melwitt | but when mriedem tried it, it blew up because of the str() call | |
| 21:19:54 | bauzas | lemme try it | |
| 21:23:28 | mriedem | str() blows up because sqla tries to do some things on __str__ | |
| 21:23:36 | mriedem | for unicode or repr type stuff | |
| 21:24:04 | mriedem | https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/engine/url.py#L90 | |
| 21:24:12 | mriedem | https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/engine/url.py#L69 | |
| 21:24:17 | mriedem | yeah it encodes the parameters | |
| 21:24:19 | mriedem | for URL encoding | |
| 21:25:40 | bauzas | mriedem: something I don't understand is why we don't change url.connection directly and stringify url ? | |
| 21:26:59 | mriedem | bauzas: you can't str(url) because of ^ | |
| 21:27:02 | melwitt | bauzas: that's what we're talking about | |
| 21:27:10 | mriedem | bauzas: what time is it in le france? | |
| 21:27:17 | zzzeek | mriedem / melwitt : the URL object has all the fields you need to make whatever string you need | |
| 21:27:35 | mriedem | zzzeek: yeah we could build a url i suppose using the parts | |
| 21:27:41 | mriedem | frankenurl | |
| 21:27:47 | mriedem | (tm) | |
| 21:28:15 | bauzas | mriedem: 23:28 | |
| 21:28:16 | melwitt | I'm still back on not understanding why it can't re-stringify the url just because we changed url.database | |
| 21:28:27 | bauzas | sec, uploading the change | |
| 21:31:00 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Avoid unnecessary lazy-loads in mutated_migration_context https://review.openstack.org/471486 | |
| 21:31:08 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Fix cell0 naming when QS params on the connection https://review.openstack.org/471487 | |
| 21:31:33 | bauzas | mriedem: melwitt: zzzeek: just lemme know why I can't just str() the object | |
| 21:31:36 | bauzas | ^ | |
| 21:31:39 | melwitt | heh, that's the same thing mriedem tried | |
| 21:32:00 | mriedem | bauzas: because https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/engine/url.py#L69 | |
| 21:32:05 | mriedem | you don't get the same thing back | |
| 21:32:10 | mriedem | you'll get URL-encoded username/password | |
| 21:32:27 | bauzas | okay, that's a small thing then | |
| 21:32:37 | bauzas | I'll rev the patch | |
| 21:40:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DNM: see what else is doing useless migration context stuff https://review.openstack.org/471491 | |
| 21:41:14 | zzzeek | bauzas: you should be able to str() it ? | |
| 21:41:46 | zzzeek | bauzas: repr() will hide the password but str() should be fine... | |
| 21:44:39 | melwitt | zzzeek: we tried that and it didn't work because it was percent-encoding colon and forward slash that were in the database connection url | |
| 21:48:00 | zzzeek | melwitt: can you show me this URL | |
| 21:48:25 | bauzas | zzzeek: eg. 'mysql+pymysql://test:abcd0123%AB@controller/nova' | |
| 21:49:00 | melwitt | zzzeek: it was in the review I linked but here ya go: reference = 'mysql+pymysql://nova:abcd0123@AB@controller/nova_cell0' actual = u'mysql+pymysql://nova:abcd0123%40AB@controller/nova_cell0' | |
| 21:49:16 | melwitt | that's one example | |
| 21:49:47 | melwitt | it changed the @ in the password to %40 | |
| 21:51:06 | bauzas | FWIW, I'm tempted to just rindex() where I can find the DB and just join what's before and after | |
| 21:51:27 | melwitt | bauzas: yeah, that's what I pasted earlier | |
| 21:51:38 | zzzeek | bauzas / melwitt OK so that first URL does go back and forth between URL / string symetrically: http://paste.openstack.org/show/611562/ however, it's not as easy to put inside a file b.c. the character is not ASCII | |
| 21:52:13 | zzzeek | a function that returns an ASCII escaped form of a SQLA URL is defintely a good function to have, in oslo.db to start and SQLA can have it too | |
| 21:53:13 | zzzeek | ah ok %40, let's try that | |
| 21:53:19 | bauzas | zzzeek: right, that's the problem we have | |
| 21:53:39 | bauzas | or we could assume it's a non-problem ? | |
| 21:54:21 | zzzeek | bauzas: well the @ sign has to be %40 because @ is syntactically part of the URL | |
| 21:54:23 | zzzeek | 'mysql+pymysql://test:abcd0123%40AB@controller/nova' | |
| 21:54:23 | zzzeek | >>> u = url.make_url('mysql+pymysql://test:abcd0123%40AB@controller/nova') | |
| 21:54:23 | zzzeek | >>> str(u) | |
| 21:54:43 | zzzeek | if we are talking about @ sign, that has to be %40 in all str representations | |
| 21:54:53 | bauzas | right | |
| 21:54:55 | zzzeek | >>> u.password | |
| 21:54:56 | zzzeek | 'abcd0123@AB' | |
| 21:55:11 | zzzeek | that works symettrically, you can put it in a conf file no problem | |
| 21:56:32 | melwitt | hmm | |
| 21:56:59 | openstackgerrit | Merged openstack/nova master: Remove Babel from requirements.txt https://review.openstack.org/468350 | |
| 21:57:16 | bauzas | that's what I wondered when I stated this a potential non-problem | |
| 21:57:24 | melwitt | yeah, so I guess to the actual database it will work fine but the user would see their password look different in the conf file after we muck with it | |
| 21:57:44 | bauzas | melwitt: we're not updating the file, right? | |
| 21:58:05 | bauzas | melwitt: it's just the connection string that is in the DB that would differ from what the operator provided | |
| 21:58:39 | bauzas | which couldn't be a problem if the service is able to connect thru the given creds even if they are not ASCII-shown | |
| 21:58:44 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Revert "Remove Babel from requirements.txt" https://review.openstack.org/471498 | |
| 21:58:50 | mriedem | sdague: ^ | |
| 21:58:56 | mriedem | babel isn't used in code, it's used in config | |
| 21:59:47 | mriedem | sfinucan: ^ also fyi | |
| 22:04:01 | bauzas | zzzeek: so, just to clarify, the fact that the operator uses special chars like in passwords isn't messed up if we URL-encode those ? | |
| 22:04:41 | zzzeek | bauzas: special chars in the password can all be represented as %XX when they are passed in | |
| 22:14:06 | bauzas | mriedem: still around ? | |
| 22:14:21 | mriedem | depends | |
| 22:14:37 | bauzas | saw the above? | |
| 22:14:43 | mriedem | i didn't follow it no | |
| 22:15:27 | bauzas | the fact that stringifying the URL object makes it RFC1738 compatible doesn't necessarly mean it messes up the connection string | |
| 22:25:44 | melwitt | it doesn't mess it up as far as, it should still work for connecting to the database, but it could store a url different than the user has in their conf file for the nova DB. that is, the password will be percent-encoded for cell0, and they could see the difference when listing cells, if listing cells is possible | |
| 22:26:46 | mriedem | listing cells is possible | |
| 22:26:52 | mriedem | nova-manage cell_v2 list_cells --verbose | |
| 22:27:00 | mriedem | dumps the cells and --verbose dumps the db connection and transport url | |
| 22:28:24 | bauzas | okay, it's becoming late to me | |
| 22:29:24 | bauzas | if we really want to have the string as close as possible with the original opt value, then I only see something like rindex or rsplit helping us | |
| 22:31:12 | bauzas | or we could construct from the URL object by getting the original username and password | |
| 22:31:19 | bauzas | melwitt: mriedem: ^ | |
| 22:31:50 | melwitt | yeah, that's what we were saying earlier. either the rreplace() equivalent or a frankenurl | |
| 22:32:32 | mriedem | ahem, that'll be $.05 please | |
| 22:32:46 | bauzas | frankenurl ? :) | |
| 22:33:21 | melwitt | the frankenurl seems more correct, if there's not danger of leaving something out while we're constructing it | |
| 22:33:53 | melwitt | collecting royalties in perpetuity | |
| 22:36:17 | mriedem | i'd think we could plug the bits into some urlparse method to generate a url and then stringify it | |
| 22:36:27 | mriedem | but i haven't looked of course | |
| 22:37:45 | mriedem | eff we have some functional test race issue now | |