Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2019-06-05
15:33:13 gtema would be however nice somehow to mark in Ansible things - please preffer this name
15:33:52 gtema so that sometime we can simply drop them and not take care of them for ever
15:48:51 mordred gtema, Shrews: remote: https://review.opendev.org/663368 Explicitly set use_direct_get to Fal
15:49:03 mordred there's the nodepool patch to allow us to do the update in sdk
15:49:18 gtema ok, good
15:49:35 openstackgerrit charlie proposed openstack/openstacksdk master: Support deleting all routes in update_router https://review.opendev.org/663369
16:38:02 Shrews mordred: ok, at a proper computer again. you got a tl;dr version for me? that's quite a bit of scrollback
16:38:27 mordred Shrews: MUST READ ALL WORDS
16:38:51 Shrews then my vote is YES to whatever the thing is
16:39:23 mordred Shrews: so - tl;dr - make default behavior in sdk (2) from the list of three things in scrollback
16:39:51 mordred Shrews: and have nodepool pass use_direct_get=False in its Connection constructor - since it's important that it have the different behavior
16:43:32 Shrews ok, i'm just going to have to read the entire sb because i don't see the connection yet
16:45:41 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Push use_direct_get support into resource layer https://review.opendev.org/663379
16:46:32 mordred Shrews, gtema: ^^ something like that (I'm 100% sure that isn't going to fully work - but that's the general idea)
16:46:52 gtema ack
16:47:40 mordred (I think we'll need to flip the default in the same patch - otherwise we're going to start listing in places we weren't - and that's goign to involve a BUNCH of requests_mock changes)
16:57:12 Shrews "oh! nodepool doesn't use filtering at all" <--- yes, this is what was confusing me
16:58:18 Shrews mordred: so if the question is "which of those 3 options sounds best?", then i'm fine with 2. I still don't see any relationship to nodepool and use_direct_get
16:58:36 Shrews because nodepool doesn't use that directly, afaict
16:58:44 mordred Shrews: well, it uses get_server
16:59:13 Shrews and doesn't get_server do the is_uuid_like thing?
16:59:25 mordred which in current behavior does the full list and then filters client-side to find the server by id because use_direct_get is False by default
16:59:36 mordred Shrews: only when use_direct_get is True
17:01:20 Shrews mordred: sorry, brain is not working i guess. so why do we need to set use_direct_get=False in nodepool if that's the default?
17:02:02 mordred Shrews: ah - because we want to swap the default in sdk because for most people filtering a list locally is much less efficient and they get grumpy
17:02:12 mordred Shrews: but before we do that, we need to make nodepool explicitly set the behavior it wants
17:02:31 Shrews ah ha! switching the default is the piece i was missing
17:02:35 gtema mordred, Shrews, what is the use_case of nodepool? is id known or could it be name?
17:02:46 mordred we know the id
17:03:01 gtema then _get will immediately get it
17:03:07 mordred but we dont want to make get calls because we might thousands of outstanding servers we're getting status on
17:03:13 gtema and not do any listing AFAIK
17:03:16 Shrews mordred: i mean, switching the default behavior seems like an API break
17:04:10 gtema find will try to get and do list if not found, but not the 'get'
17:04:13 mordred gtema: in the resource layer, as things are currently written, that is right. but in the cloud layer, get_server will filter the list - which is what we want in nodepool
17:04:28 gtema ah, you mean there. Got it
17:04:55 mordred because in nodepool what we want is to do one list call shared by all thousand threads that are trying to launch a server in paralell, and then filter that list in each thread
17:05:09 mordred otherwise cloud operators really hate us
17:05:12 gtema yupp
17:06:03 mordred Shrews: yes - I think it is - but I think it's a behavior change that would more closely match expectations of casual users. I'd be willing to bet money that nobody _other_ than nodepool is relying on the filtered-list behavior
17:07:31 Shrews mordred: where does nodepool filter servers? I don't see that anywhere
17:07:47 mordred Shrews: it doesn't - it uses wait_for_server
17:08:44 mordred Shrews: wait_for_server itself uses get_server
17:08:59 gtema ok, leaving you on that note. CU tomorrow
17:09:04 mordred have fun
17:09:09 gtema thks
17:09:12 mordred which does filter_list / get_entity on search_servers
17:10:26 Shrews oh, for some reason i was thinking of a different form of filtering.
17:10:58 Shrews well, flipping the behavior is your call as PTL. Easy enough to change for nodepool
17:11:40 Shrews it would be nice behavior, i agree
17:12:54 Shrews mordred: +2'd the nodepool change
17:13:26 mordred \o/
18:54:56 openstackgerrit Brian Haley proposed openstack/python-openstackclient master: Support IPv6 addresses better https://review.opendev.org/524420
20:13:23 efried mordred: Do we not defer message format interpolation in logging in openstacksdk?
20:13:47 mordred efried: well - I mean, that's the intent - that doesn't mean we always do it
20:14:08 efried okay, I'll keep looking for examples. The one I happend to land on first, doesn't.
20:14:21 efried " since verify=False".format(full_name=self.full_name))
20:14:21 efried "Turning off SSL warnings for {full_name}"
20:14:21 efried self.log.debug(
20:14:42 mordred yeah - it's entirely possible there are more places like that
20:37:53 efried mordred: I am unsatisfied with the current handling of "You've got a section called [foo] but when I try to load Adapter opts from it, I get an exception."
20:39:09 efried (2) Register [foo] and register ksa adapter opts in it, but f up the settings
20:39:09 efried (1) Register the [foo] group, but don't register ksa adapter opts in it
20:39:09 efried For example, I am able to trigger this code path in the following two ways:
20:39:38 efried For (1) I think we should make the Connection refuse to talk to foo.service_type
20:39:51 efried ...but otherwise that shouldn't be an exception.
20:40:00 mordred agree
20:40:02 efried For (2) I think we should let the exception raise.
20:40:27 mordred yes - but we should be sure to provide good errors when we do
20:40:43 efried Today, for both paths, we allow the Connection with default adapter settings.
20:40:49 efried That seems... not good.
20:41:36 efried I would be able to manage this, but hamstrung by the fact that we don't want to import oslo.config on the prod side.
20:41:55 efried Options:
20:42:01 mordred we can import it behind protection
20:42:04 efried 1) import oslo.config in from_conf
20:42:18 efried because if you're using from_conf presumably you've got it installed somewhere
20:42:36 efried 2) string scrape exception class.__name__ or similar (ew)
20:42:44 mordred yup! and it should be fine to import it there - or even just a try import block at top
20:43:13 mordred nah - I think it's totally fine to import oslo.config for the purposes of from_conf - we just dont' want people who are using sdk for ansible playbooks to need to install oslo.config
20:43:28 efried okay, I'll go with it. Thanks
20:43:45 efried um, one more thing
20:43:51 efried How do I prevent Connection?
20:43:55 efried for a given service?
20:44:29 mordred sure thing! also - we should maybe consider triggering an attempted auth or something in from_conf - since we otherwise only create the adapters on-demand (for things like OSC) - but I imagine in nova getting the error earlier rather than later would be better
20:45:12 mordred efried: that's a great question ... we have a has_{service} structure so that config can assert that we don't have a service - but it's not plumbed in to the Connection/adapter code
20:45:17 mordred now that you mention it - it should be :)
20:45:26 mordred I can make a patch for that
20:48:43 efried mordred: My dumb approach would be to make ConnectionMeta.__new__ return None if not any(key.startswith(service_type + '_') for key in self.config)
20:48:58 efried ...but only if we came from_conf
20:49:34 mordred efried: so - in Connection, we have has_service which does: if not self.config.config.get('has_%s' % service_key, True): - which we use in other places
20:50:51 mordred I'm thinking perhaps codify that a little bit, add a has_service on CloudRegion, then in from_conf we can add an else condition on if project_name not in conf: which sets has_{service_type} to false for that service type
20:50:52 efried o, so dumb second iteration, ConnectionMeta.__new__ returns None if not has_service(service_type) ?
20:51:02 mordred then in __new__ ... yeah, does that ^^
20:51:57 mordred that way it's a broadly applicable flag people can use to fully disable something if they need to for whatever reason
20:52:17 mordred (although as a followup we could get fancy and return an object that throws an exception in its __getattr__)
20:52:29 efried yeah, was just thinking that
20:52:45 mordred because "None object has no attribute servers" is always a crappy error message
20:52:51 efried yeah
20:53:16 efried fancier, make the exception message describe why we disabled the service
20:53:22 mordred yeah!

Earlier   Later