| *** VW has joined #craton | 03:00 | |
| *** Tamayo has quit IRC | 03:32 | |
| openstackgerrit | Merged openstack/craton master: Adding wrapper functions to tools https://review.openstack.org/449230 | 04:51 |
|---|---|---|
| openstackgerrit | Merged openstack/craton master: Ensure JSON responses result from failure https://review.openstack.org/447580 | 04:51 |
| jimbaker | antonym, read only users, yes. you can see some of the ideation here: https://blueprints.launchpad.net/craton/+spec/craton-rbac-support | 04:56 |
| jimbaker | but in essence, the basic CRUD ops will be split out as potential roles | 04:56 |
| jimbaker | with any level of granularity on a given resourfce | 04:56 |
| *** acabot has quit IRC | 08:11 | |
| *** VW has quit IRC | 10:54 | |
| *** VW has joined #craton | 10:54 | |
| *** openstackgerrit has quit IRC | 11:33 | |
| *** VW has quit IRC | 12:50 | |
| *** VW has joined #craton | 12:50 | |
| *** VW has quit IRC | 13:08 | |
| thomasem | o/ | 13:28 |
| sulo | o/ | 13:30 |
| fsaad | morning guys | 13:37 |
| *** VW has joined #craton | 13:39 | |
| git-harry | sulo: what's going on with those client patches you have in progress? I seem to remember them being mentioned in yesterday's meeting. Do you need any assistance? | 14:01 |
| thomasem | Thanks for the feedback on the JSON Path patch, folks. Going to fix that up. | 14:06 |
| sulo | git-harry: ill fix them up sometime this week | 14:07 |
| thomasem | sulo: I was planning to add documentation in a patch following this one. Do you feel strongly that it ought to be in this patch? | 14:07 |
| sulo | thomasem: cool, na its good | 14:07 |
| thomasem | Since this is already a good 700+ line change, I was hoping not to bloat the review with docs stuff, too. | 14:07 |
| thomasem | Okay, cool | 14:07 |
| sulo | if you already had that in mind, i am fine. | 14:07 |
| sulo | doing it separately | 14:08 |
| thomasem | Awesome. Thanks! | 14:08 |
| anonymike | o/ | 14:09 |
| thomasem | git-harry: hmmm, it looks like jsonpath-rw is printing an invalid path... $.bar.[*] | 14:10 |
| *** Tamayo has joined #craton | 14:10 | |
| thomasem | test = jspath.parse('foo.bar[*]') | 14:10 |
| thomasem | When it sets the Fields object containing 'foo' to Root(), it winds up causing the array Slice to be sectioned off as a member. | 14:11 |
| thomasem | https://gist.github.com/thomasem/42c40d95b9a8f214cf98a9057bf14e9e | 14:12 |
| thomasem | So, that's going to cause some trouble if it's doing that. | 14:12 |
| thomasem | Heh, it's incorrect to begin with: | 14:13 |
| thomasem | In [13]: str(test) | 14:13 |
| thomasem | Out[13]: 'foo.bar.[*]' | 14:13 |
| thomasem | test = jspath.parse('foo.bar[*]') | 14:13 |
| git-harry | thomasem: Are you saying "foo.bar.[*]" != "foo.bar[*]"? | 14:16 |
| thomasem | That's correct. At least MySQL doesn't like it. | 14:16 |
| thomasem | It may be the same from a specification perspective (though I'm not wild about the dual meaning), but MySQL definitely doesn't like it. | 14:17 |
| git-harry | thomasem: yeah, the docs say they're equivalent https://github.com/kennknowles/python-jsonpath-rw#jsonpath-syntax | 14:18 |
| thomasem | So, I think the implementation I already have is going to be the most successful here. Though, I do not like it much either. The alternative being massaging what's produced by jsonpath-rw every time an array is involved to make it into something MySQL likes. | 14:18 |
| git-harry | thomasem: this would also need to be documented because any user's will get an error even though it looks valid | 14:21 |
| thomasem | That's okay. I'm planning to specify the subset of JSON Path that is supported. | 14:21 |
| thomasem | in the documentation | 14:21 |
| thomasem | Since MySQL only supports https://dev.mysql.com/doc/refman/5.7/en/json-path-syntax.html | 14:21 |
| thomasem | And because jsonpath-rw doesn't support the double-asterisk, I have to drop that, too. | 14:22 |
| thomasem | MySQL 5.7, I should say. I can't speak to the latest. | 14:22 |
| * thomasem sighs | 14:23 | |
| thomasem | Least common denominator here, I'm afraid. | 14:23 |
| *** VW_ has joined #craton | 14:30 | |
| *** VW_ has quit IRC | 14:31 | |
| *** VW_ has joined #craton | 14:31 | |
| thomasem | fwiw, the most common way to specify arrays is to just add the slice/index specification right after the key, and not to add a period. At least that's what I've seen over the past week digging around. | 14:32 |
| thomasem | That jsonpath-rw is printing it always with a period before it is a bit weird to me. | 14:33 |
| *** VW has quit IRC | 14:34 | |
| git-harry | I suspect there will be lots of fun edge cases causing all sorts of pain from these differences. | 14:35 |
| thomasem | Yeah. It makes one wonder if we ought to drop the feature altogether for.. Elasticsearch. :D | 14:36 |
| thomasem | The subset of support in MySQL coupled with some fun idiosyncracies... | 14:36 |
| thomasem | Not to mention how MySQL actually supports something that it appears jsonpath-rw does not - the double asterisk. | 14:37 |
| thomasem | I'm hoping I covered at least the 80% case, though. | 14:37 |
| *** openstackgerrit has joined #craton | 14:38 | |
| thomasem | Pushed an update https://review.openstack.org/#/c/443941/30 | 14:39 |
| thomasem | Ahhhh, interesting. I guess openstackgerrit was out. I wonder if it logged back in as a result of my patch, but didn't go on to push the notification. Sounds like a bug. | 14:39 |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: JSON Path-like querying for variables https://review.openstack.org/443941 | 14:42 |
| thomasem | There it goes | 14:42 |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: JSON Path-like querying for variables https://review.openstack.org/443941 | 14:43 |
| thomasem | I'm sorry! | 14:43 |
| jimbaker | thomasem, makes sense. when i was first looking at pulling jsonpath-rw into a related bit of work, my biggest concern was the possibility of two separate implementations with slightly different support | 14:44 |
| jimbaker | this appears to be the case | 14:45 |
| jimbaker | much like different regex implementations etc | 14:45 |
| thomasem | Yep. Everyone's got their own flavor and we're the suckers that have to glue it together. | 14:47 |
| thomasem | :P | 14:47 |
| anonymike | lol :/ | 14:47 |
| jimbaker | right, sorry about that. perhaps we are pushing the boundaries of sql + json at this time. certainly *mysql* + json :) | 14:48 |
| jimbaker | anonymike, were you able to make progress on using the python client and the ES bulk api? | 14:49 |
| jimbaker | python client for craton | 14:49 |
| jimbaker | anonymike, also thanks for the work on the wrappers, glad to see that merged in! | 14:50 |
| anonymike | Not much, I totally switched to working on the fields bug. I have the docker compose stuff to get craton and two elastic nodes built | 14:50 |
| anonymike | and a script to pull things down from craton after populating with fake data | 14:50 |
| anonymike | I was writing the piece to send it to elastic search when we had our meeting yesterday | 14:51 |
| jimbaker | anonymike, cool. we can share in a (throwaway) github repo | 14:51 |
| anonymike | Sounds good | 14:52 |
| jimbaker | but the fields stuff is the most important of the work that was not in progress on monday, so right prioritization | 14:52 |
| jimbaker | hopefully pretty clear what it looks like in the client | 14:52 |
| jimbaker | re valid json path selectors, i don't know how much we can borrow from thomasem's recent changes to his patch here | 14:53 |
| anonymike | Cool :) I'm trying to get more familiar with everything so these tasks don't take so long lol | 14:53 |
| anonymike | yeah I was going to hold off on that for a while | 14:53 |
| jimbaker | so basic field selector support, then look at json path selectors support? sounds workable | 14:54 |
| jimbaker | could divide into two patches that way too | 14:54 |
| anonymike | Great | 14:54 |
| jimbaker | anonymike, also earlier thomasem and i were thinking about this in terms of regexes | 14:56 |
| anonymike | the fields path? | 14:57 |
| *** VW_ has quit IRC | 14:57 | |
| anonymike | er fields filter | 14:57 |
| jimbaker | against the keys. it is quite possible that both forms are valid | 14:57 |
| *** VW has joined #craton | 14:57 | |
| jimbaker | anonymike, most of the value of the jsonpath is filtering on values after all | 14:58 |
| jimbaker | so it's a very good reason not to get too caught up in a specific implementation choice | 14:58 |
| anonymike | right | 14:59 |
| jimbaker | is all i'm pointing out. so breaking this out is highly valid. hopefully no confusion here on the choices we need to look at | 14:59 |
| jimbaker | at the end of the day, that's why the specific data requirements from bjoern are so useful | 15:00 |
| anonymike | Nope, I don't think so. I'll get something out that filters out top level keys in the json response and hold off on nested keys... correct? | 15:01 |
| jimbaker | anonymike, +1 | 15:01 |
| thomasem | Would appreciate another quick round of reviews when folks have a chance https://review.openstack.org/#/c/443941/31 | 15:08 |
| thomasem | oop: https://review.openstack.org/#/c/443941/32 | 15:08 |
| thomasem | #32 :) | 15:08 |
| jimbaker | thomasem, plan to do so | 15:10 |
| thomasem | Huzzah | 15:12 |
| *** VW has quit IRC | 15:22 | |
| *** VW has joined #craton | 15:23 | |
| thomasem | fsaad: Are we still using https://etherpad.openstack.org/p/craton-sprint-planning for planning, or are we going elsewhere? | 15:34 |
| thomasem | jimbaker: ^^ | 15:34 |
| fsaad | I think we agreed last time on using craton-meetings? | 15:34 |
| thomasem | Cool | 15:34 |
| jimbaker | +1. certainly feel free to pull in from that etherpad of course | 15:35 |
| fsaad | on that note, please self drive as I may not make it in time due to a meeting reschedule that will overlap | 15:35 |
| jimbaker | lots of useful info | 15:35 |
| fsaad | I'm going to follow up with y'all right after though. | 15:35 |
| thomasem | I know we had agreed that when we're on StoryBoard we're just going to share that and use it for this. | 15:35 |
| thomasem | Instead of etherpads | 15:35 |
| thomasem | Basically prioritizing a pre-existing backlog | 15:36 |
| * thomasem can't wait | 15:36 | |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: JSON Path-like querying for variables https://review.openstack.org/443941 | 15:37 |
| thomasem | Rebased | 15:37 |
| *** klindgren_ has joined #craton | 15:37 | |
| *** klindgren has quit IRC | 15:38 | |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: Add documentation for JSON Path-like variable searching https://review.openstack.org/450861 | 15:39 |
| anonymike | oh yeah, are we still next in line for migration to storyboard? | 15:42 |
| anonymike | that seems to take a while | 15:42 |
| jimbaker | thomasem, see my comment re something like craton-get v1/devices?vars=bar.baz:47 | 15:43 |
| jimbaker | let's work on this in a separate bug | 15:43 |
| jimbaker | but i don't like forever bugs | 15:43 |
| jimbaker | confused phrasing | 15:44 |
| thomasem | Wait, did you see something wrong with integer values? | 15:44 |
| jimbaker | should have stated: let's work on this in a separate bug, because i don't like forever bugs | 15:44 |
| jimbaker | no, it's the devices endpoint | 15:44 |
| jimbaker | integers, etc, work just fine | 15:44 |
| thomasem | Oh! Yeah. That's definitely a separate bug. Should be a one-liner. | 15:45 |
| thomasem | Well... more like 2 or 3, but yeah. | 15:45 |
| jimbaker | yeah. plus some testing | 15:45 |
| thomasem | I'm writing up the bug now. | 15:45 |
| jimbaker | so let's just treat separately | 15:45 |
| thomasem | Yeah, and I can just add the test class for the other stuff. | 15:45 |
| thomasem | Since it's generic | 15:45 |
| jimbaker | thanks! | 15:45 |
| thomasem | cool | 15:45 |
| jimbaker | anonymike, i did talk with kendall nelson yesterday, and we are in the next batch | 15:46 |
| jimbaker | re storyboard | 15:46 |
| jimbaker | so soonish | 15:46 |
| anonymike | :) | 15:46 |
| anonymike | so I've since moved on and just started working in the directory, but I cannot get they pythonclient to install system wide | 15:47 |
| jimbaker | given how the migration works, should be fine. we just need to cut over to the new ui from launchpad when we are ready. so nothing we need to do | 15:47 |
| anonymike | the* | 15:47 |
| jimbaker | in the meantime | 15:47 |
| thomasem | https://bugs.launchpad.net/craton/+bug/1676932 | 15:47 |
| openstack | Launchpad bug 1676932 in craton "Devices endpoint does not support filtering by variables" [Undecided,New] | 15:47 |
| jimbaker | thomasem, can you look across the board at what devices does and does not support in that bug? | 15:48 |
| jimbaker | another reason why i wanted the split to be honest... | 15:48 |
| jimbaker | actually whoever works on that bug should do that. it might be thomasem ... | 15:48 |
| thomasem | jimbaker: updated | 15:48 |
| thomasem | "Also, let's audit what all's missing from /v1/devices compared to /v1/hosts and address here." | 15:49 |
| thomasem | I'd like to get the JSON Path docs out first, personally. | 15:49 |
| thomasem | Then work on that. | 15:49 |
| thomasem | I want to be pretty explicit in docs, including identifying limitations of the feature. | 15:49 |
| thomasem | So, want to spend some time on it. | 15:50 |
| git-harry | I'd rather see the more general problem addressed that just adding vars support to devices | 15:50 |
| thomasem | We can, of course, do a separate task from this to simply audit what the differences are and issue bugs for them. | 15:50 |
| thomasem | If there are any others. | 15:50 |
| git-harry | thomasem: I guess what I'm saying is that shouldn't be necessary. The model should be more all or nothing similar to how the variables endpoint is handled. | 15:51 |
| thomasem | jimbaker: Thinking about it... what do you mean "look across the board"? | 15:56 |
| thomasem | What sorts of things are you suspecting aren't supported? | 15:56 |
| *** VW has quit IRC | 15:58 | |
| jimbaker | thomasem, umm, well when something basic is missing like var search, i get worried, that's all | 15:58 |
| jimbaker | and in terms of prioritization, obviously the jsonpath needs to be done. that's why i asked we do this in a separate bug :) | 15:58 |
| jimbaker | time for that meeting | 15:59 |
| anonymike | vidyo crashed :( restarting | 16:03 |
| *** VW has joined #craton | 16:04 | |
| *** VW_ has joined #craton | 16:58 | |
| *** VW__ has joined #craton | 17:01 | |
| *** VW_ has quit IRC | 17:01 | |
| *** VW has quit IRC | 17:02 | |
| jimbaker | git-harry, sulo, anything you want to add to https://review.openstack.org/#/c/443941/ ? i believe thomasem has addressed your concerns and this patch is ready to be added to master | 17:02 |
| anonymike | jimbaker thomasem: why do we want the fields args comma separated instead of space separated like the current implementation | 18:31 |
| anonymike | I implemented the solution to handle both cases that Ian suggested, but I'm curious why spaces wouldn't work | 18:32 |
| anonymike | https://bugs.launchpad.net/python-cratonclient/+bug/1675410 for reference | 18:32 |
| openstack | Launchpad bug 1675410 in Craton's Python Client "Add fields selector to <resource>-show commands" [Critical,New] - Assigned to Michael Porras (mporras6856) | 18:32 |
| jimbaker | anonymike, i think we should support something like | 18:35 |
| jimbaker | craton host-show --fields=spec1,spec2 --fields=spec3 | 18:36 |
| jimbaker | but certainly open to other suggestions | 18:37 |
| anonymike | hmm okay... That'd be just like doing craton host-show --fields=spec1,spec2,spec3? | 18:37 |
| jimbaker | anonymike, yes | 18:37 |
| anonymike | okay | 18:37 |
| anonymike | not a cli wizard but is that standard? repeating flags in a command? | 18:38 |
| jimbaker | anonymike, yes, fairly typical | 18:38 |
| anonymike | interesting | 18:38 |
| jimbaker | not certain for openstack CLIs | 18:39 |
| anonymike | cool I'll do that. thanks for the clarification | 18:39 |
| jimbaker | but argparse has good support for this | 18:39 |
| jimbaker | anonymike, see the append action - https://docs.python.org/3.5/library/argparse.html#action | 18:42 |
| jimbaker | for comma separation, we have to do the flattening ourselves, i don't believe argparse has specific support here | 18:43 |
| anonymike | Yeah, I used the link Ian provided | 18:43 |
| jimbaker | the only liability in fact with comma separation is if the field spec has a comma in it. then it becomes a mess :) | 18:44 |
| jimbaker | in terms of any parsing | 18:44 |
| anonymike | would that ever be the case? :/ | 18:44 |
| jimbaker | well, we did say key could be any valid unicode... | 18:46 |
| jimbaker | vs just a python identifier, which could still use unicode | 18:46 |
| jimbaker | it does come up. basically anytime you think that someone does only X, they actually do a bit more than that | 18:47 |
| jimbaker | and then just standard stuff. such as having names being drawn from other sources than ascii. so a recent bug in monasca because it could not support instances with chinese names | 18:48 |
| thomasem | I think --fields has to be the last option if it's space delimited. | 18:48 |
| thomasem | Also, spaces in paths or JSON strings could cause some trouble. | 18:49 |
| jimbaker | well if it's space delimited, usually it's passed in like so | 18:49 |
| jimbaker | --fields "baz bar foo" | 18:49 |
| jimbaker | so the shell can give it to you as one arg | 18:49 |
| thomasem | Gotcha | 18:49 |
| thomasem | Well, going with commas is more in-line with what the API supports, too. | 18:49 |
| thomasem | space separate feels a bit clunky, but maybe that's just me. | 18:50 |
| thomasem | separated* | 18:50 |
| jimbaker | yeah. as usual, i like to raise questions | 18:50 |
| thomasem | For example | 18:50 |
| jimbaker | comma separated is so much more convenient. unless it doesn't work | 18:50 |
| jimbaker | thomasem, but i think the better answer is as follows | 18:50 |
| jimbaker | if you have a comma in the field name, you will have to use the alternative json spec version we have talked about | 18:51 |
| anonymike | can I change it to JUST comma separated and not worry about space | 18:51 |
| jimbaker | anonymike, yeah, i think that's fine | 18:51 |
| anonymike | cool | 18:51 |
| jimbaker | because having a space in the name is surely the more common thing to see | 18:51 |
| anonymike | agreed | 18:51 |
| thomasem | That may be a bug with the existing implementation, actually in Craton API. | 18:52 |
| jimbaker | corner cases! | 18:52 |
| jimbaker | when we will ever get rid of them??? ;) | 18:52 |
| thomasem | https://github.com/openstack/craton/blob/12b31e49ac1e9fe135497d59f3f2a67b5c1bcafc/craton/db/sqlalchemy/api.py#L268 | 18:52 |
| thomasem | When we specify the valid characters? :D | 18:53 |
| thomasem | and validate | 18:53 |
| jimbaker | thomasem, hence my valid python identifiers musing... | 18:54 |
| thomasem | Right | 18:54 |
| jimbaker | someone already did the specification, and there's even code we can use | 18:54 |
| jimbaker | perhaps a better way to do this is the following | 18:55 |
| jimbaker | a valid python identifier can be used without quoting | 18:55 |
| jimbaker | if quoted, any valid unicode key can be used | 18:56 |
| jimbaker | so this is similar to how it's treated in sql | 18:56 |
| jimbaker | as it is, i doubt we really allow arbitrary keys with '=', ':', '.', ',' in them, at least not in every position. i don't know how jsonpath intersects with this in terms of key spec either | 18:58 |
| jimbaker | so another possible cleanup. not super critical, but out there | 18:59 |
| thomasem | Haven't really tried. | 19:00 |
| *** VW has joined #craton | 19:01 | |
| *** VW has quit IRC | 19:03 | |
| *** VW__ has quit IRC | 19:04 | |
| *** VW has joined #craton | 19:04 | |
| *** VW has quit IRC | 19:41 | |
| *** VW has joined #craton | 19:42 | |
| *** VW_ has joined #craton | 20:15 | |
| *** VW_ has quit IRC | 20:16 | |
| *** VW_ has joined #craton | 20:16 | |
| *** VW has quit IRC | 20:18 | |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: Add documentation for JSON Path-like variable searching https://review.openstack.org/450861 | 20:49 |
| *** VW has joined #craton | 20:58 | |
| *** VW has quit IRC | 20:58 | |
| *** VW has joined #craton | 20:59 | |
| *** VW_ has quit IRC | 21:01 | |
| anonymike | woo so I think I got --fields working with host list/show for both table and json.. Gonna go run, then apply to all the other shells and write some tests | 21:25 |
| jimbaker | anonymike, awesome stuff, thanks! | 21:27 |
| *** VW_ has joined #craton | 21:30 | |
| *** VW has quit IRC | 21:30 | |
| *** VW_ has quit IRC | 21:35 | |
| *** VW has joined #craton | 21:35 | |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: Add documentation for JSON Path-like variable searching https://review.openstack.org/450861 | 21:41 |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: Add documentation for JSON Path-like variable searching https://review.openstack.org/450861 | 21:48 |
| thomasem | Alright. Would appreciate feedback on that ^^ | 21:50 |
| thomasem | Just some light documentation. We can, of course, go into further detail. | 21:51 |
| openstackgerrit | Thomas Maddox proposed openstack/craton master: Add documentation for JSON Path-like variable searching https://review.openstack.org/450861 | 21:51 |
| thomasem | Anyway, I'm outta here. Have a lovely evening/day! | 21:55 |
| *** VW_ has joined #craton | 23:27 | |
| *** VW has quit IRC | 23:30 | |
| *** VW_ has quit IRC | 23:31 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!