| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-09 | |||
| 12:44:59 | jaypipes | bauzas: I'm not following you... | |
| 12:45:13 | bauzas | jaypipes: sorry, I'm trying to explain | |
| 12:45:48 | bauzas | jaypipes: I'm not really *against* using module functions | |
| 12:45:56 | bauzas | jaypipes: it's just another design pattern | |
| 12:46:11 | bauzas | jaypipes: but the problem I see with using that pattern is about | |
| 12:46:32 | bauzas | 1/ we could have a long list of functions | |
| 12:46:43 | bauzas | 2/ each function is set globally by python | |
| 12:47:21 | bauzas | which means for 2/ you need to make sure to not use a global variable | |
| 12:47:47 | jaypipes | bauzas: you need to make sure not to use global variables in staticmethods as well... | |
| 12:48:24 | bauzas | not really | |
| 12:48:30 | bauzas | you can use class variables | |
| 12:49:09 | bauzas | the main problem is about the variables scopes | |
| 12:49:09 | jaypipes | bauzas: sorry, I guess I just don't see the difference | |
| 12:50:31 | Tengu | hello! | |
| 12:51:23 | Tengu | small question: I'd like to get some stats regarding OVS using collectd, I guess I'll need to configure some SNMP plugin in collectd? or are there better way? (note: not sure this is the right channel… ?) | |
| 12:52:07 | bauzas | jaypipes: sorry that's where I have some problems explaining | |
| 12:52:11 | jaypipes | Tengu: hi :) definitely not the right channel :) | |
| 12:52:22 | jaypipes | Tengu: are you an OpenStack user? | |
| 12:52:24 | bauzas | jaypipes: honestly, https://softwareengineering.stackexchange.com/questions/171296/staticmethod-vs-module-level-function is what I want to explain | |
| 12:52:48 | bauzas | there are not so much differences in between using the static deco and just a module-scoped function | |
| 12:52:56 | Tengu | jaypipes: yup - sorry, OVS in openstack env - should have precised. | |
| 12:53:16 | bauzas | it's more about what kind of maintainability you want to get, and if you want to make sure to not have problems later | |
| 12:53:53 | jaypipes | bauzas: these functions are operating on the database, not on the objects. Which is why I want to have them in separate module-level functions. | |
| 12:54:26 | jaypipes | Tengu: I think the best thing would be to ask your question on the openstack-operators@ mailing list. | |
| 12:54:40 | Tengu | jaypipes: hmm ok. | |
| 12:55:31 | jaypipes | bauzas: also... did you read the part in the question that says "I've googled this question, and it seems there's some general agreement that module-level functions are preferred over static methods because it's more pythonic."? :) | |
| 12:56:22 | bauzas | jaypipes: well, the dude is wrong when he's saying ". Static methods have the advantage of being bound to its class" | |
| 12:56:36 | bauzas | because static methods are *unbound* AFAIK | |
| 12:56:45 | bauzas | I was rather pointing the best answer | |
| 12:57:35 | bauzas | tbc, I see the CRUD ops as just being against a specific table | |
| 12:57:39 | jaypipes | bauzas: I think we'll just have to agree to disagree on this one. It's a code style preference, nothing more. | |
| 12:57:48 | bauzas | in that case, I just feel those should be defined as static | |
| 12:58:08 | bauzas | jaypipes: agreed, I was about to say the same, it's just a pattern discussion | |
| 12:58:14 | bauzas | nothing wrong from your side | |
| 12:58:18 | bauzas | just stylistic | |
| 12:58:34 | bauzas | that said, I did not -1d for that | |
| 12:58:51 | bauzas | I rather pointed the problem of explicitelly telling which columns to return | |
| 12:59:12 | bauzas | from a maintenability PoV, that is concerning me | |
| 13:00:33 | jaypipes | bauzas: and I responded that I prefer to have the columns be listed explicitly for the reason I outlined in my response to you. I gave you a specific example of what happens when you add a field to the ORM model and that field is a relationship() result and all of a sudden your queries that use that ORM model bloat up unnecessarily. | |
| 13:03:06 | bauzas | jaypipes: hum, seems like I missed that point then | |
| 13:03:13 | bauzas | if so, I apologize | |
| 13:03:26 | bauzas | jaypipes: in https://review.openstack.org/#/c/509025/2/nova/objects/resource_provider.py@405 ? | |
| 13:03:42 | jaypipes | yes | |
| 13:03:49 | bauzas | the Gerrit formatting messed up most of your comment | |
| 13:03:56 | jaypipes | bauzas: the comment I made on Oct 6 at 2:50pm | |
| 13:04:09 | jaypipes | bauzas: ugh, yeah :( | |
| 13:05:04 | bauzas | hah, I thought you were telling me *why* to not use ORM objects, which I don't disagree | |
| 13:05:38 | bauzas | I didn't noticed you were giving me an explanation about why it's important to be telling which fields to return | |
| 13:06:40 | jaypipes | bauzas: no worries. /me grabs breakfast and coffee now :) | |
| 13:07:36 | bauzas | jaypipes: /me needs to look again at your comment then | |
| 13:24:52 | openstackgerrit | Eric Fried proposed openstack/nova master: nova.utils.get_ksa_adapter() https://review.openstack.org/488137 | |
| 13:24:54 | efried | mriedem ^ | |
| 13:25:06 | efried | I worked on it all weekend | |
| 13:28:02 | mriedem | bauzas: i went through the bottom change in the "target host during cold migration" series again https://review.openstack.org/#/c/408955/ | |
| 13:28:03 | mriedem | fresh -1 | |
| 13:28:22 | efried | jaypipes Will you get a chance to skim https://review.openstack.org/510244 before the scheduler meeting? | |
| 13:28:51 | bauzas | mriedem: ack, needs to review as well | |
| 13:35:25 | dansmith | jaypipes: sdague: bauzas: This and the one right behind it are needed to unblock rally's gate, FYI: https://review.openstack.org/#/c/510203 | |
| 13:36:25 | bauzas | dansmith: looking | |
| 13:36:55 | sdague | dansmith: +2 | |
| 13:37:03 | sdague | dansmith: also, ug multiple sort queries | |
| 13:37:21 | dansmith | sdague: yeah, :/ | |
| 13:37:27 | sdague | that's mostly a good way to use no indexes | |
| 13:37:44 | dansmith | that | |
| 13:37:50 | dansmith | is another ibm special feature | |
| 13:37:57 | dansmith | but, the ship has sailed | |
| 13:39:22 | stephenfin | gibi: Single comment on https://review.openstack.org/#/c/483324/ | |
| 13:40:36 | gibi | stephenfin: thanks. | |
| 13:41:16 | gibi | stephenfin: I think that part of the code makes sure that the bdm is marked deleted in db | |
| 13:41:36 | stephenfin | gibi: But we still have the object reference that we can show? | |
| 13:41:38 | gibi | stephenfin: as we are at a last step of instance deletion | |
| 13:42:02 | gibi | stephenfin: yes we have the in memory version of that bdm | |
| 13:42:16 | gibi | stephenfin: or somebody could query it out form the db with read_deleted=True | |
| 13:42:58 | gibi | stephenfin: I can move the bdm.destroy() after the delete.end notification if that looks more logical to you | |
| 13:45:29 | mnaser | i'm trying to figure out from the commits if the placement database connection is something that is no longer being used? | |
| 13:46:05 | mnaser | it looks like it was reverted a while back.. but there's an open patch to add it again (that's pretty old): https://review.openstack.org/#/c/362766/ ? | |
| 13:49:23 | dansmith | mnaser: it is used by placement | |
| 13:49:27 | dansmith | er, wait | |
| 13:50:04 | mnaser | dansmith: https://review.openstack.org/#/q/39fb302fd9c8fc57d3e4bea1c60a02ad5067163f not sure if that helps you cobble things up together but it's not helping me much | |
| 13:50:06 | mnaser | (im reviewing the puppet change and seeing if we should deprecate that option or not) | |
| 13:50:29 | dansmith | mnaser: so we introduced some things a little too early and backed up a few steps, | |
| 13:50:46 | dansmith | but it's forward-looking not something that is going to go away | |
| 13:51:15 | dansmith | mnaser: we put something back in after that revert I think, just a sec | |
| 13:52:52 | dansmith | or maybe we never put it back | |
| 13:53:44 | dansmith | mnaser: well, anyway, "Deprecate" is probably the wrong word for the puppet patch, but it's confusing | |
| 13:54:25 | krypto | hello all any idea why windows vm created with 4 vcpu flavor shows only 2 vcpu inside it? I can see that in vm configuration its 4 cpu but inside the guest its just 2 | |
| 13:55:47 | mriedem | dansmith: looks like unit test failures in https://review.openstack.org/#/c/506416/ are real | |
| 13:55:59 | dansmith | mriedem: yep I know | |
| 13:56:25 | mnaser | dansmith: i think we'll add a warning to mention that it's currently un-used (not deprecated) till https://review.openstack.org/#/c/362766/ is merged | |
| 13:56:37 | mriedem | we never put the placement api stuff back in | |
| 13:56:39 | mriedem | after the revert | |
| 13:56:44 | mriedem | s/api/db/ | |
| 13:57:02 | edleafe | Scheduler subteam meeting in 3 minutes in #openstack-meeting-alt | |
| 13:57:24 | mnaser | mriedem so should we deprecate that option in puppet-nova or just warn that it's not used until it gets added again? | |
| 13:57:41 | dansmith | mriedem: I thought we had put an alternate connection url conf option in, that let you separate if not, but apparently we didn't | |
| 13:57:47 | mriedem | mnaser: i'd probably just say it's not used | |
| 13:58:12 | mriedem | otherwise we have to coordinate deprecations | |
| 13:58:24 | mriedem | and once puppet drops it, we might add it back in :) | |
| 13:58:37 | mnaser | alright cool, i'll leave a note mentioning to drop the warning once https://review.openstack.org/#/c/362766/ merges (if it ever does that is)? | |
| 13:58:58 | mriedem | we haven't put serious thought into what the migration / split out is going to look like | |