*** VW has quit IRC | 00:09 | |
*** david-lyle has joined #craton | 00:09 | |
*** david-lyle has quit IRC | 00:20 | |
*** VW has joined #craton | 00:29 | |
*** VW_ has joined #craton | 00:33 | |
*** VW has quit IRC | 00:33 | |
*** VW_ has quit IRC | 00:36 | |
*** Syed__ has quit IRC | 02:05 | |
*** harlowja has quit IRC | 02:53 | |
sulo | ok here is the patch for search by label: https://review.openstack.org/#/c/432283/ | 12:30 |
---|---|---|
*** VW has joined #craton | 13:06 | |
thomasem | Reviewing | 13:41 |
thomasem | sulo: so this converts to an exists subquery? | 14:04 |
thomasem | `AND (EXISTS (SELECT 1 \nFROM labels \nWHERE devices.id = labels.device_id AND labels.label = :label_1)` | 14:04 |
thomasem | instead of what was previously a join that was causing the issue | 14:04 |
thomasem | or at least having some issue :P | 14:04 |
sulo | thomasem: yes, it will be a any query .. instead of a join | 14:27 |
thomasem | Cool | 14:44 |
thomasem | refactoring project variables to use generic, I'm debating how we want to handle the root/admin permissions on those calls. | 14:44 |
sulo | yeah it would have been nice to have some rbac impl there but given what we have | 14:45 |
sulo | i guess it makes sense to allow users in the project access to vars | 14:45 |
thomasem | Yeah, but we'd still want to lock down updating/deleting vars, no? | 14:46 |
thomasem | Or shall we just leave that for the RBAC impl? | 14:46 |
sulo | i would just leave it tbh, i dunno its worth all the work to implement any code for this now | 14:46 |
sulo | espcially gien jimbaker is quite close to the rabc patch | 14:47 |
sulo | close to finishing | 14:47 |
thomasem | Okay, cool. Then, yeah, I'm cool w/ that. | 14:47 |
thomasem | Thanks, sulo! | 14:47 |
*** jovon has joined #craton | 15:22 | |
thomasem | Ah, bummer... more snags because project_id is a UUID instead of int, like the generic variables tests assume. | 15:32 |
jimbaker | sulo, https://review.openstack.org/#/c/432355/ | 15:45 |
jimbaker | sulo, i assume that the current API for DELETE (which git-harry did reproduce as part of his refactoring yesterday) was not your intent | 15:46 |
jimbaker | but see that change for more | 15:46 |
git-harry | Yes, I listed this, in the variables spec I wrote, as being something as counter-intuitive but I didn't want to change the API as part of the refactor | 15:48 |
sulo | jimbaker: git-harry: was it taking a dict with k:v then just deleting the k before ? | 15:53 |
sulo | that sounds about right, iirc | 15:53 |
jimbaker | sulo, no it took v:k | 15:54 |
sulo | value : key ?? | 15:54 |
git-harry | more like _: k | 15:54 |
jimbaker | yes | 15:54 |
jimbaker | correct | 15:54 |
jimbaker | the value was ignored | 15:54 |
sulo | jimbaker: yeah i think i was just iterating on the keys and nuking them regardless of what the value was | 15:55 |
jimbaker | no one expected the dictionary inversion! | 15:55 |
jimbaker | ;) | 15:55 |
sulo | anyway this is the right way to go about doing it | 15:55 |
sulo | i dunno why i did that than just passing list and nuking | 15:55 |
sulo | :/ | 15:55 |
jimbaker | sulo, git-harry and i both understood the intent. although i had to dive in the code, the errors reported by the API were interesting | 15:56 |
jimbaker | sulo, no worries | 15:56 |
jimbaker | anyway, if we can fast track that review - i need that for the client change so we can support deletes | 15:56 |
jimbaker | one thing i wanted to discuss yesterday but we ran out of time | 15:57 |
jimbaker | in general, the vars spec that harry put together is great in terms of recommending JSON PATCH. but i think partial DELETE and partial PUT to represent better how variables are a mapping would work better | 15:58 |
jimbaker | ie, let's keep it the way it is now, assuming the DELETE fix i just proposed | 15:58 |
jimbaker | for nonpartial usage of PUT and DELETE, there is a simple solution: just fully PUT the desired object. or fully specify the list of keys to DELETE | 16:00 |
git-harry | Yup, as I said in the spec, there are many ways to do this. The point about the current implementation of PUT and DELETE is that it goes against the RFCs and API-WG guidelines. | 16:02 |
*** acabot has quit IRC | 16:09 | |
thomasem | To what degree do we intend to follow the API-WG guidelines? | 16:24 |
thomasem | Because, yeah... it's a bit weird right now | 16:24 |
sigmavirus | thomasem: git-harry, I think, wants us to follow them to the letter | 16:29 |
thomasem | Okay. What about others? :) | 16:29 |
thomasem | I'm trying to get a general gauge for folks' opinions. Not suggesting anything, btw. | 16:30 |
sigmavirus | I think they're more or less a good idea but that we have some time before we need to meet every guideline | 16:30 |
sigmavirus | And that the work we're doing now to match every guideline is slowing down priorities by causing merge conflicts | 16:30 |
*** Syed__ has joined #craton | 16:31 | |
git-harry | I'm not sure you're following a standard unless you're following it to the letter. | 16:31 |
git-harry | I don't think the merge conflicts are a big deal, rebasing is not that hard | 16:31 |
git-harry | and that is separate. | 16:31 |
sigmavirus | git-harry: it is not hard but it is time consuming | 16:32 |
sigmavirus | And it's especially time consuming for those of us not properly allowed to ever focus on one project | 16:32 |
git-harry | One is if we should change things, the other is when | 16:32 |
jimbaker | right. so guidelines are just tht | 16:33 |
thomasem | So, it's a goal to get there, but we are willing to focus on priorities at the expense of technical debt in that area. The hesitations I have are related to deprecation strategies and how long we HAVE to keep the technical debt around. That being said, if we don't remain relevant and move quick enough, it won't matter anyway. | 16:33 |
jimbaker | if following guidelines means that the only practical way to use REST with variables is via JSON PATCH, i don't think that's realistic | 16:33 |
thomasem | Swap out some periods for question marks in my message, lol. | 16:33 |
jimbaker | i don't even see it as tech debt | 16:33 |
thomasem | Wasn't meaning to assert. | 16:33 |
sigmavirus | thomasem: so there have been 0 releases of craton | 16:34 |
thomasem | Right | 16:34 |
sigmavirus | So we have no deprecation strategy that we are required to follow | 16:34 |
jimbaker | +1 | 16:34 |
sigmavirus | We have no users because craton is not presently meeting their requirements | 16:34 |
sigmavirus | And so we don't have to worry about breaking early adopters | 16:34 |
sigmavirus | And for young projects, they tend to move fast and break things | 16:35 |
jimbaker | +100 | 16:35 |
sigmavirus | If we care about deprecation strategies we could start looking into microversioning early on the right way (not the glance way) and then our deprecation strategy will be solved | 16:35 |
thomasem | Mmmm, yeah. I mean, we can do it in a stable way so as to not cause too much disruption once we do release. | 16:35 |
thomasem | Yeah, that's the reason I'm bringing this up | 16:35 |
jimbaker | i don't think we are going to get a lot of pushback by changing APIs in the next few months. we definitely will not get pushback if we change our APIs now :) | 16:36 |
sigmavirus | To be fair, getting started with microversioning now would be good | 16:36 |
jimbaker | we do need to preserve the data, through good migrations. that's a given | 16:36 |
sigmavirus | But that's a lot of upfront effort | 16:36 |
jimbaker | right now we have more important priorities | 16:37 |
sigmavirus | Also having rolling upgrades early on would be good | 16:37 |
sigmavirus | exactly | 16:37 |
sigmavirus | Luckily we already use alembic so we're farther along for RU than most other "openstack" projects | 16:37 |
jimbaker | agreed on rolling upgrades. and we got some practice as a team on going through complex migrations | 16:38 |
jimbaker | more complex than i actually anticipate needing to do | 16:38 |
jimbaker | (even thought of submitting a talk on alembic based on that experience, before we wisely "rolled" that effort back!) | 16:38 |
jimbaker | (for pycon) | 16:38 |
jimbaker | so the actual tech debt we have is | 16:39 |
jimbaker | 1. we should support JSON PATCH where it makes sense; but in example continue to provide a reasonable alternative path that better corresponds to how variables are used in their lifecycle (these alternatives are not tech debt) | 16:40 |
jimbaker | 2. support microversions | 16:40 |
thomasem | I'm not going to plant my feet over it. I do see a risk associated if we go rapidly changing things after release to try and bring it into compliance with the expected guidelines. IF we don't version, any tooling that consumes this API will also have to change along with it. | 16:40 |
jimbaker | thomasem, but early customers are going to be using the tooling | 16:40 |
thomasem | I'm not sure I follow what you're trying to convey by that statement? | 16:41 |
jimbaker | thomasem, they are going to use the client/CLI | 16:41 |
thomasem | Okay, so we're going to call our API unstable? | 16:41 |
thomasem | and suggest client/CLI use as opposed to writing things against the API itself. | 16:41 |
thomasem | Until we cut a stable API? | 16:42 |
jimbaker | we are going to minimize changes to the API | 16:42 |
jimbaker | but until we implement microversioning, yes, i think that's best. right now, i see the real changes are additions however - and adjustments where there were obvious bugs | 16:42 |
thomasem | Sure. As long as we're explicit about the API limitations, I'm fine with that. | 16:43 |
jimbaker | this stuff comes up in the python lang/library dev, to wit, why are you breaking my code by fixing that bug ;) | 16:43 |
jimbaker | i should be able to depend on that bug | 16:43 |
jimbaker | guido van rossum has a simple answer. "no." | 16:43 |
thomasem | Haha, of course. I've just become more of a stable than volatile guy over the years due to things like that. So, that's where my mind's at about it. | 16:44 |
jimbaker | now if it's a better design - and that can be hard to distinguish from a bug - that's another matter altogether | 16:45 |
thomasem | So, I hope I'm introducing some healthy dissension and not an extreme. | 16:45 |
thomasem | Well, I've never passed a body to a DELETE call before. | 16:45 |
thomasem | So, that's different. :) | 16:45 |
jimbaker | for example iirc list comprehensions leak their iterator variable in at least python 2 (don't recall if changed in 3) | 16:45 |
jimbaker | thomasem, yeah, but how does one delete a variable then? | 16:46 |
thomasem | I believe it was fixed in 3. | 16:46 |
thomasem | partial PUT or PATCH | 16:46 |
jimbaker | how do we distinguish a deletion from setting to null/None? | 16:46 |
thomasem | in the PUT scenario you expect a full list of variables you want to be set and update accordingly on our side. | 16:46 |
jimbaker | if we do PUT | 16:46 |
thomasem | in the PATCH scenario, you set the 'path' to the variable you wish to remove | 16:46 |
thomasem | and "op": "remove" | 16:47 |
jimbaker | thomasem, but the expectation is that there will be many variables, not just a few | 16:47 |
thomasem | So? | 16:47 |
jimbaker | i agree that PATCH is better here | 16:47 |
jimbaker | so apparently we disagree :) | 16:47 |
thomasem | Are you talking thousands? | 16:47 |
thomasem | hundreds? | 16:47 |
jimbaker | certainly could be hundreds | 16:48 |
jimbaker | now we have put the burden of managing that on the client | 16:48 |
thomasem | Okay. Just thinking if it's programmatically doing it, you'd GET the variables, update the list, then PUT with the new list. | 16:48 |
jimbaker | also consider what it means with namespaces | 16:48 |
thomasem | update the dict* | 16:48 |
jimbaker | which presumably should allow for some isolation | 16:48 |
thomasem | Right... I suppose then you'd have an even easier time? | 16:49 |
jimbaker | so PUT is update the dict | 16:49 |
jimbaker | sure. except i would like to use namespaces like they are in python | 16:49 |
jimbaker | not in java | 16:49 |
jimbaker | should have plenty of flexibility in their usage, including flattening | 16:50 |
jimbaker | by import | 16:50 |
thomasem | So, let's do an example | 16:50 |
git-harry | It would be nice to capture this conversation in the spec | 16:50 |
thomasem | I'm sorry, git-harry, you're right. | 16:50 |
jimbaker | git-harry, fortunately it's being logged | 16:50 |
jimbaker | so we can discuss, and spec-ify from this | 16:51 |
git-harry | okay, just want to make sure there is an outcome | 16:51 |
thomasem | Yeah, beers after work. | 16:51 |
thomasem | :D | 16:51 |
jimbaker | http://eavesdrop.openstack.org/irclogs/%23craton/latest.log.html#t2017-02-10T15:45:59 | 16:52 |
jimbaker | is the approx starting point | 16:53 |
thomasem | So, I want to better understand the imagined implementation for namespaces as it relates... So, how could several layers deep be interacted with via the API, let's say `maintenance.foo.bar` | 16:53 |
thomasem | as the namespace | 16:53 |
thomasem | How would you want to DELETE a variable from that namespace? | 16:54 |
* sulo payes attention to the channel, seems like there is some interesting convo going on here | 16:55 | |
sulo | ugh i cant even type | 16:55 |
* sulo goes back to day dreaming | 16:55 | |
Syed__ | haha | 16:56 |
jimbaker | so first, i'm proposing maintenance/foo/bar | 16:57 |
jimbaker | because / is now allowed in ansible, but is valid in URLs | 16:57 |
jimbaker | NOT | 16:57 |
thomasem | So, curl -X DELETE http://localhost:8080/hosts/1/variables/maintenance/foo/bar ...? | 16:57 |
jimbaker | uhh, most certainly not | 16:58 |
thomasem | haha, okay, good | 16:58 |
jimbaker | more for stuff like var search, imports, that sort of thing. think url query params | 16:58 |
thomasem | Gotcha, okay | 16:58 |
thomasem | so ?namespace=maintenance/foo/bar | 16:58 |
jimbaker | the whole idea is that we can implement namespaces today | 16:58 |
jimbaker | just try it | 16:58 |
*** VW_ has joined #craton | 16:58 | |
jimbaker | it becomes convention; and the client has to do its own filtering | 16:58 |
jimbaker | ok, although this discussion is great, i have a meeting to go to | 16:59 |
thomasem | Oh, no worries! We'll pick it up at some point. I think there are indeed more pressing matters immediate-term. | 16:59 |
thomasem | Thanks for the discussion and let's pick it up later? | 17:00 |
*** VW_ has quit IRC | 17:00 | |
*** VW__ has joined #craton | 17:00 | |
*** VW has quit IRC | 17:02 | |
*** VW__ has quit IRC | 17:02 | |
*** VW has joined #craton | 17:02 | |
*** VW_ has joined #craton | 17:03 | |
*** VW has quit IRC | 17:03 | |
*** harlowja has joined #craton | 17:05 | |
*** harlowja has quit IRC | 17:05 | |
*** harlowja has joined #craton | 17:05 | |
*** VW_ has quit IRC | 17:23 | |
*** VW has joined #craton | 17:24 | |
*** VW has quit IRC | 18:08 | |
*** VW has joined #craton | 18:11 | |
jimbaker | sulo, so device search by label continues to work | 18:27 |
jimbaker | i guess this means we can save for the time being, even though i still think we should move to a vars approach | 18:27 |
jimbaker | i will take a look at that change; please look at the delete support since i would like to get the client/cli variables support finished up | 18:28 |
jimbaker | https://review.openstack.org/#/c/432355/ needs reviews :) | 18:28 |
thomasem | I will review it right after lunch. | 18:30 |
thomasem | brb | 18:30 |
*** VW has quit IRC | 18:32 | |
*** rainya has joined #craton | 19:03 | |
*** rainya has quit IRC | 19:03 | |
*** tojuvone has quit IRC | 19:07 | |
*** VW has joined #craton | 19:29 | |
toan | jimbaker: are you here sir? | 19:58 |
jimbaker | hi | 19:58 |
toan | hey jimbaker, do you have a few minutes to chat about rpc plan? | 19:59 |
jimbaker | sure thing | 19:59 |
jimbaker | on vidyo? | 19:59 |
toan | cool, could you join my vid room? | 19:59 |
toan | ant, kevin, dusty and farid are there as well | 19:59 |
farid | thomasem: ^ | 20:00 |
jimbaker | thomasem, ^^^ | 20:00 |
*** VW has quit IRC | 20:26 | |
*** jovon has quit IRC | 20:32 | |
*** VW has joined #craton | 20:51 | |
thomasem | farid, toan: I was in a collision on my way back from lunch and just finally got home. | 20:54 |
toan | thomasem: hope you're ok | 20:54 |
thomasem | I'm okay, just a little shaken up, so I'm going to step away. | 20:54 |
thomasem | Wanted to let y'all know what happened. | 20:54 |
*** VW has quit IRC | 20:54 | |
toan | ok, thanks. pls just take care of your situation | 20:55 |
thomasem | Thanks, toan | 20:55 |
farid | thomasem: oh no! | 20:55 |
farid | thomasem: take care man, talk to you later | 20:56 |
*** VW has joined #craton | 20:57 | |
jimbaker | thomasem, glad you are ok, but do take that time | 21:12 |
*** VW has quit IRC | 21:25 | |
*** VW has joined #craton | 21:29 | |
*** VW has quit IRC | 21:57 | |
*** VW has joined #craton | 22:10 | |
*** VW_ has joined #craton | 22:11 | |
*** VW has quit IRC | 22:11 | |
*** VW has joined #craton | 22:55 | |
*** VW has quit IRC | 22:58 | |
*** VW has joined #craton | 22:58 | |
*** VW_ has quit IRC | 22:59 | |
*** VW has quit IRC | 22:59 | |
*** VW has joined #craton | 23:00 | |
*** VW has quit IRC | 23:17 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!