| *** shakayumi has quit IRC | 00:31 | |
| *** nos_ has joined #openstack-metering | 00:54 | |
| *** nos_ has quit IRC | 00:54 | |
| *** nosnos has joined #openstack-metering | 00:55 | |
| *** changbl has quit IRC | 00:55 | |
| *** Ruetobas has joined #openstack-metering | 00:59 | |
| *** yjiang5 is now known as yjiang5_away | 01:00 | |
| *** changbl has joined #openstack-metering | 01:00 | |
| *** giroro_ has joined #openstack-metering | 01:01 | |
| *** Ruetobas has quit IRC | 01:04 | |
| *** giroro_ has quit IRC | 01:04 | |
| *** Ruetobas has joined #openstack-metering | 01:11 | |
| *** shaneduan is now known as shaneduan[afk] | 01:19 | |
| *** flwang has quit IRC | 01:33 | |
| *** litong has joined #openstack-metering | 01:39 | |
| *** giroro_ has joined #openstack-metering | 01:56 | |
| *** Ruetobas has quit IRC | 01:58 | |
| *** Ruetobas has joined #openstack-metering | 01:59 | |
| *** d34dh0r53 has joined #openstack-metering | 01:59 | |
| *** giroro_ has quit IRC | 02:00 | |
| *** d34dh0r53 has quit IRC | 02:01 | |
| *** Ruetobas has quit IRC | 02:07 | |
| *** shaneduan[afk] is now known as shaneduan | 02:08 | |
| *** Ruetobas has joined #openstack-metering | 02:10 | |
| *** Ruetobas has quit IRC | 02:14 | |
| *** flwang has joined #openstack-metering | 02:26 | |
| *** shaneduan is now known as shaneduan[afk] | 02:30 | |
| *** nosnos_ has joined #openstack-metering | 02:47 | |
| *** shang has joined #openstack-metering | 02:47 | |
| *** nosnos has quit IRC | 02:49 | |
| *** nosnos_ has quit IRC | 02:55 | |
| *** nosnos has joined #openstack-metering | 02:55 | |
| *** Fengqian has joined #openstack-metering | 02:56 | |
| *** anteaya has quit IRC | 02:58 | |
| *** shaneduan[afk] is now known as shaneduan | 03:05 | |
| *** shaneduan is now known as shaneduan[afk] | 03:09 | |
| *** Guest16331 has quit IRC | 03:59 | |
| *** lillie has joined #openstack-metering | 04:00 | |
| *** lillie is now known as Guest72665 | 04:00 | |
| *** SergeyLukjanov has joined #openstack-metering | 04:25 | |
| openstackgerrit | Sandy Walsh proposed a change to openstack/ceilometer: Reject duplicate events https://review.openstack.org/44191 | 04:33 |
|---|---|---|
| *** boris-42 has joined #openstack-metering | 04:51 | |
| *** evanjfraser has quit IRC | 04:55 | |
| *** openstack has joined #openstack-metering | 14:54 | |
| *** openstackgerrit has joined #openstack-metering | 14:55 | |
| jd__ | dhellmann: around? | 15:04 |
| dhellmann | pong | 15:04 |
| jd__ | I'm thinking about Jay's email on SQL | 15:04 |
| jd__ | I'm really worried | 15:05 |
| dhellmann | how so? | 15:05 |
| *** d34dh0r53 has joined #openstack-metering | 15:05 | |
| jd__ | the plan was OK a week ago, but discussin this last hours with ttx makes me realize that may be a bit late in the process | 15:05 |
| jd__ | +g s/this/during these/ | 15:06 |
| dhellmann | so you think pagination is going to slip to the next release? | 15:06 |
| dhellmann | I would rather do that than rush something broken in. This is one of those features I don't think we understood well enough when we started out. | 15:07 |
| jd__ | it's pretty much already negociated with ttx that pagination will be rescheduled | 15:07 |
| dhellmann | sure, that works for me | 15:08 |
| dhellmann | we should make sure we have a slot at the summit to hash out the details | 15:08 |
| eglynn | dhellmann, gorc: hopefully my comments on https://review.openstack.org/44908 re. the on_behalf_of field have shed some light? | 15:08 |
| dhellmann | eglynn: I've been looking at sandywalsh_'s dupe handling changeset, but I'll get back on that one next | 15:09 |
| eglynn | dhellmann: thanks! | 15:09 |
| jd__ | dhellmann: can you review https://review.openstack.org/#/c/44292/ ? | 15:09 |
| jd__ | eglynn: too ^ | 15:09 |
| *** d34dh0r53 has quit IRC | 15:10 | |
| sandywalsh_ | eglynn jd__ I can look at both of those now too | 15:10 |
| eglynn | jd__: yep, on it | 15:10 |
| jd__ | thanks sandywalsh_ | 15:10 |
| jd__ | ok then we have another big problem IMHO | 15:10 |
| * jd__ mode "branle-bas de combat" | 15:10 | |
| dhellmann | jd__: I see you added a new migration, so that should be a quick review | 15:10 |
| eglynn | jd__: pagination slipping, what about the switch back from alembic to sa-migrate? | 15:11 |
| jd__ | eglynn: I don't know, that's what I'm raising the issue | 15:11 |
| eglynn | jd__: a-ha, k | 15:11 |
| dhellmann | I thought we agreed to use both but allow new sqlalchemy-migrate migrations for this release, and force the move to alembic in icehouse | 15:11 |
| dhellmann | so we don't have to undo the alembic stuff | 15:12 |
| dhellmann | jd__: "commotion combat"? is that another idiom? :-) | 15:12 |
| * eglynn just catching up with that "Wait a minute..." thread on the ML (email client out of action all morning due to VPN issues ...) | 15:13 | |
| jd__ | dhellmann: well http://fr.wiktionary.org/wiki/branle-bas_de_combat indicates that it comes from "clearing the decks before beginning the combat on a ship", it seems to be http://en.wikipedia.org/wiki/Action_Stations in English | 15:14 |
| dhellmann | jd__: ok, that makes more sense :-) | 15:14 |
| * jd__ learned something | 15:14 | |
| jd__ | so the other problem I would like to get your attention onto is https://review.openstack.org/#/c/43851/ | 15:15 |
| jd__ | migrations are broken with PostgreSQL | 15:15 |
| jd__ | so we can't activate devstack-gate on Ceilometer right now | 15:16 |
| jd__ | and I wish we could considering all the patches coming in for SQL | 15:16 |
| dhellmann | indeed | 15:16 |
| dhellmann | that's a bug, though, right? so it can happen between h3 and rc1? | 15:16 |
| dhellmann | s/happen/be fixed/ | 15:16 |
| jd__ | it's a bug and my fix is https://review.openstack.org/#/c/44691/ though you may want to read Svetlana comments too | 15:17 |
| dhellmann | I'm running out of browser tabs for these reviews :-) | 15:17 |
| jd__ | is there a limit on browser tab? :) | 15:20 |
| dhellmann | I guess we'll find out. | 15:21 |
| sandywalsh_ | eglynn: where in the test is the auth set up so the on-behalf-of checks can be done? (I assume you would have to set up a user A with access to project B?) | 15:22 |
| dhellmann | sandywalsh_: eglynn and I talked about that a little while ago, and he pointed me toward test_get_recorded_alarm_history_segregation | 15:22 |
| eglynn | sandywalsh_: yep, what dhellmann said | 15:23 |
| dhellmann | although that tests access failure, is there one that tests for success? | 15:23 |
| eglynn | test_get_recorded_alarm_history_rule_change_on_behalf_of | 15:24 |
| eglynn | IIUC the question correctly ... | 15:24 |
| *** shanewang1 has joined #openstack-metering | 15:24 | |
| *** shanewang1 has left #openstack-metering | 15:25 | |
| sandywalsh_ | hmm, should be an explicit test on _sanitize_query I think ... it's complex enough to warrant. Otherwise it's functionality is getting hidden as side effects in other tests. | 15:25 |
| sandywalsh_ | *its | 15:25 |
| eglynn | sandywalsh_: OK, I'll do that now | 15:25 |
| dhellmann | sandywalsh_: yeah, I asked eglynn to add some more explicit tests for the sorting in another changeset, too | 15:26 |
| eglynn | dhellmann, sandywalsh_: will work on both in the same new changeset | 15:26 |
| sandywalsh_ | cool | 15:26 |
| dhellmann | eglynn: that on-behalf-of test looks like it picks a new random uuid as the auth credentials for the query, what sets that new id up as having access to the other project's data? | 15:26 |
| sandywalsh_ | a unit test shouldn't really span the method being tested | 15:26 |
| sandywalsh_ | if we follow that assumption for all of our tests, the tests get better coverage and act as better documentation. | 15:27 |
| eglynn | dhellmann: a-ha, true that ... I'm over-using the overridden auth | 15:27 |
| sandywalsh_ | s/span/go outside of/ | 15:27 |
| eglynn | dhellmann: I'll fix that also now | 15:27 |
| dhellmann | eglynn: cool, thanks | 15:27 |
| dhellmann | sandywalsh_: +2 with one question on https://review.openstack.org/#/c/44191/ | 15:28 |
| dhellmann | I think the answer to my question is "no" :-) | 15:28 |
| sandywalsh_ | dhellmann: thanks ... reading | 15:29 |
| dhellmann | I'm not sure why we made that API deal with a list of events like that. | 15:29 |
| dhellmann | jd__: approved https://review.openstack.org/#/c/44292/ | 15:32 |
| jd__ | \o/ | 15:33 |
| sandywalsh_ | dhellmann: responded ... good question. One I've been debating a lot recently :) | 15:35 |
| dhellmann | jd__: I'm not sure what to do with the migrations question | 15:35 |
| dhellmann | sandywalsh_: it may have been a misunderstanding of something on my part early on that we just haven't removed yet | 15:35 |
| sandywalsh_ | dhellmann: howso? | 15:36 |
| sandywalsh_ | jd__: phooey, lost my place in the migration :) now I need 014_ | 15:36 |
| jd__ | sandywalsh_: hihi | 15:37 |
| jd__ | dhellmann: I think the best we can do is fix the bug wrt PostgreSQL; anyway we'll have too since infra is going to enable devstack-gate on us soon and we'll be blocked if we don't pass it | 15:39 |
| dhellmann | sandywalsh_: of the idea that the incoming message queue might give us more than one thing at a time to process | 15:40 |
| dhellmann | jd__: which review should I be looking at first? https://review.openstack.org/#/c/44539/ ? | 15:40 |
| sandywalsh_ | dhellmann: right ... if it's coming from oslo.rpc, no. But I can see other "services" publishing more than one. The event trigger pipeline for one. | 15:40 |
| openstackgerrit | A change was merged to openstack/ceilometer: alarm api: rename counter_name to meter_name https://review.openstack.org/44292 | 15:41 |
| *** shaneduan[afk] is now known as shaneduan | 15:41 | |
| dhellmann | sandywalsh_: good point | 15:41 |
| sandywalsh_ | dhellmann: but that won't affect service.py. But the low level code still needs to handle more than one event at a time. Honestly, I could go either way on it. Keep it simple at 1 event at a time and optimize later if needed. | 15:42 |
| sandywalsh_ | pie/cake | 15:42 |
| jd__ | dhellmann: I don't know :( | 15:42 |
| jd__ | dhellmann: mine is simple, maybe not correct wrt the "sync of the state of database after migration and models" but seems to work locally on my devstack | 15:43 |
| jd__ | dhellmann: sshturm's one is more intrusive | 15:43 |
| dhellmann | sandywalsh_: what you've got now is fine, we'll fix it if it turns out to be broken :-) | 15:44 |
| dhellmann | "eventually correct" | 15:44 |
| dhellmann | jd__: ok, less intrusive seems good :-) | 15:44 |
| sandywalsh_ | agreed ... we could go round and round on this forever | 15:45 |
| jd__ | eventually correct since we don't have devstack-gate :-( | 15:45 |
| dhellmann | I'd like the mirantis folks to talk to us about this database work they're doing before we land all of the rest of those patches -- the alembic thing is making me more cautious | 15:45 |
| dhellmann | sandywalsh_: yeah, I wasn't suggesting a change, just asking in case I was missing some detail about how it worked | 15:45 |
| openstackgerrit | Julien Danjou proposed a change to openstack/python-ceilometerclient: alarm: rename counter_name to meter_name https://review.openstack.org/45076 | 15:46 |
| sandywalsh_ | dhellmann: gotcha. sensible question. | 15:46 |
| openstackgerrit | Sandy Walsh proposed a change to openstack/ceilometer: Reject duplicate events https://review.openstack.org/44191 | 15:47 |
| sandywalsh_ | * just a migration numbering change | 15:48 |
| sandywalsh_ | jd__: are we that concerned with faster when N=1 (and = 0 99.9% of the time)? :) | 15:51 |
| *** changbl has joined #openstack-metering | 15:51 | |
| jd__ | sandywalsh_: no, it was just for the sake of it | 15:51 |
| sandywalsh_ | jd__: interesting construct though .. mental note :) | 15:51 |
| dhellmann | jd__: I don't understand why you removed the drop_constraint() call in https://review.openstack.org/#/c/44691/1/ceilometer/storage/sqlalchemy/alembic/versions/2c3ccda5a3ad_fix_uniq_name.py | 15:52 |
| jd__ | sandywalsh_: I'm more concerned about the code around that doesn't seem to accomplish anything (or I miss the point really) | 15:52 |
| dhellmann | jd__: the old change had any dispatcher that detected an exception preventing the rest of the dispatchers from receiving the message | 15:53 |
| dhellmann | the new version lets all dispatches process the event, and makes a list of the "duplicates" then raises an exception in one place to nack the message | 15:53 |
| sandywalsh_ | yep | 15:54 |
| dhellmann | although now that I say that out loud, I wonder if we want to nack if all of the dispatchers think it is a dupe? shouldn't we just ignore dupes? | 15:54 |
| dhellmann | s/all/any | 15:54 |
| dhellmann | to be idempotent? | 15:54 |
| sandywalsh_ | dhellmann: that's what's happening. It's only raising if some other error occured | 15:54 |
| dhellmann | aha, right, I forgot you had different error types | 15:54 |
| sandywalsh_ | right, and that'll be needed for dead-letter | 15:55 |
| dhellmann | time for lunch, bbiab | 15:55 |
| *** dhellmann is now known as dhellmann_ | 15:55 | |
| jd__ | _ means lunch? | 15:56 |
| jd__ | dhellmann_: the drop_constraints() was failling and broke the transaction so the next call would fail too | 15:57 |
| jd__ | dhellmann_: maybe that's no the proper fix :( | 15:58 |
| jd__ | dhellmann_: if I read the current code correctly, there's a try/except around record_events() so all dispatchers received the message anyway | 15:59 |
| jd__ | sandywalsh_: ^ | 15:59 |
| *** dina_belova has quit IRC | 16:00 | |
| sandywalsh_ | jd__: in the dispatchers? | 16:00 |
| jd__ | sandywalsh_: in service.py | 16:00 |
| *** Ruetobas has quit IRC | 16:01 | |
| sandywalsh_ | jd__: not any more | 16:01 |
| sandywalsh_ | it's down in the dispatchers now | 16:01 |
| jd__ | there's still a chance a dispatch raise suddenly outside its try/except no? | 16:01 |
| jd__ | +er | 16:02 |
| jd__ | too many typos, I'm getting tired :) | 16:02 |
| sandywalsh_ | :) yes, then we fall back to the last-raise-wins behavior, which isn't the preferred way. | 16:03 |
| *** Ruetobas has joined #openstack-metering | 16:04 | |
| jd__ | is it last-raise-wins or first-raise-loses? :) | 16:04 |
| jd__ | but ok I get it | 16:05 |
| sandywalsh_ | I'm sure we'll be revisiting all this again. It's the most important part of the whole process. | 16:05 |
| jd__ | I think so | 16:08 |
| *** SergeyLukjanov has quit IRC | 16:08 | |
| jd__ | but I think it'll be good enough for now anyway, approved | 16:08 |
| sandywalsh_ | thanks jd__ | 16:09 |
| *** Ruetobas has quit IRC | 16:09 | |
| *** Ruetobas has joined #openstack-metering | 16:14 | |
| *** boris-42 has quit IRC | 16:14 | |
| *** shaneduan is now known as shaneduan[afk] | 16:15 | |
| *** openstackgerrit has quit IRC | 16:16 | |
| *** openstackgerrit has joined #openstack-metering | 16:16 | |
| openstackgerrit | A change was merged to openstack/ceilometer: Reject duplicate events https://review.openstack.org/44191 | 16:26 |
| *** shaneduan[afk] is now known as shaneduan | 16:30 | |
| openstackgerrit | A change was merged to openstack/ceilometer: Network: process metering reports from Neutron https://review.openstack.org/43892 | 16:43 |
| *** boris-42 has joined #openstack-metering | 17:08 | |
| *** tasdomas is now known as tasdomas_afk | 17:20 | |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: Move alarm defaults to the API https://review.openstack.org/45085 | 17:35 |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: 'and' and 'or' operations for alarms combination https://review.openstack.org/42832 | 17:35 |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: Add alarm combination API https://review.openstack.org/41971 | 17:35 |
| *** shaneduan is now known as shaneduan[afk] | 17:40 | |
| dhellmann_ | eglynn: I didn't realize on_behalf_of was supposed to be a hidden argument. I like just making it explicit, which it seems is your current plan? | 17:40 |
| eglynn | dhellmann_: well the intent was to keep it hidden, but make the error handling neater | 17:46 |
| eglynn | dhellmann_: easiest to just push my current wip to explain ... | 17:46 |
| openstackgerrit | Eoghan Glynn proposed a change to openstack/ceilometer: Add query support to alarm history API https://review.openstack.org/44908 | 17:46 |
| eglynn | dhellmann_: ^^^ | 17:46 |
| eglynn | bbiab, need to eat ... | 17:46 |
| *** eglynn is now known as eglynn-is-hungry | 17:46 | |
| *** krtaylor has quit IRC | 17:50 | |
| *** yjiang5_away is now known as yjiang5 | 17:53 | |
| *** shaneduan[afk] is now known as shaneduan | 18:03 | |
| openstackgerrit | Terri Yu proposed a change to openstack/ceilometer: Add group by statistics in API v2 https://review.openstack.org/44130 | 18:04 |
| *** dina_belova has joined #openstack-metering | 18:11 | |
| *** SergeyLukjanov has joined #openstack-metering | 18:11 | |
| *** dina_belova has quit IRC | 18:16 | |
| sandywalsh_ | Seeing pep8 and hacking violations getting past jenkins | 18:23 |
| terriyu | hmm, Jenkins caught my pep8 error earlier this morning | 18:23 |
| openstackgerrit | A change was merged to openstack/ceilometer: Fixes service startup issue on Windows https://review.openstack.org/44961 | 18:24 |
| sandywalsh_ | yeah, some are getting caught. The #comment ones are leaking through and HACKING import ordering are getting missed. | 18:29 |
| *** eglynn-is-hungry has quit IRC | 18:31 | |
| gordc | thomasm: you can probably hold off on the clean up of code -- maybe wait for someone else to comment. don't want to make you do unneeded work. | 18:43 |
| dhellmann_ | terriyu: ping? | 18:46 |
| *** dhellmann_ is now known as dhellmann | 18:46 | |
| terriyu | hi dhellmann | 18:46 |
| dhellmann | I'm looking at the new version of the group by changes | 18:46 |
| terriyu | ok | 18:47 |
| dhellmann | how do you feel about changing that "g" argument in the API to "groupby"? | 18:47 |
| dhellmann | For the query, we expect it to be repeated a lot, so a short name is "nicer" (and "q" is a common alias for query in web forms) | 18:47 |
| dhellmann | but the grouping should have fewer repetitions and "g" isn't very descriptive | 18:48 |
| terriyu | you mean, the URL will look like http://localhost:8777/v2/meters/instance/statistics?groupby=user_id ? | 18:48 |
| dhellmann | right | 18:48 |
| terriyu | seems like a good idea to me | 18:48 |
| dhellmann | or maybe even ?groupby=user_id&groupby=resource_id | 18:48 |
| terriyu | I was just following the example in the wiki | 18:48 |
| terriyu | https://wiki.openstack.org/wiki/Ceilometer/blueprints/api-group-by#Design_example | 18:49 |
| * dhellmann looks | 18:49 | |
| dhellmann | hmm | 18:50 |
| dhellmann | I don't know if that's meant to be "do it this way" or "I didn't want to type out the groupby argument" :-) | 18:51 |
| dhellmann | I suppose g is consistent with q, but we do have "period" as well | 18:51 |
| terriyu | didn't jd__ write that part? | 18:51 |
| dhellmann | maybe, I'm not certain | 18:52 |
| * dhellmann wonders if jd__ is still online | 18:52 | |
| terriyu | if "g" is not standard, I like "groupby" better | 18:52 |
| dhellmann | afaik, there isn't a "standard" for this, so I would like to go with the longer form | 18:52 |
| terriyu | speaking of the example in the wiki, I don't think the output will look like | 18:53 |
| terriyu | {[ | 18:53 |
| terriyu | { "m1.tiny": { "min": 1, "max": 1, "avg": 1, "sum": 1 }, | 18:53 |
| terriyu | { "m1.tiny": { "min": 1, "max": 1, "avg": 1, "sum": 1 } }, ... | 18:53 |
| terriyu | at least not the way I currently wrote my patch | 18:53 |
| terriyu | I think it will be something like | 18:54 |
| terriyu | [{"groupby": {"metadata.flavor": "m1.tiny"}, "min": 1, "max": 1, ...}, {"groupby": {"metadata.flavor": "m1.tiny"}, "min": 1, "max": 1, ...}, ...] | 18:56 |
| terriyu | gordc: I posted a new version of my groupby API patch and am discussing it with dhellmann | 18:57 |
| dhellmann | terriyu: yeah, the output in the wiki looks wrong -- it's not grouping the results | 18:58 |
| * gordc likes groupby as well. | 18:58 | |
| dhellmann | terriyu: the output in the next section by asalkeld looks more correct | 18:58 |
| dhellmann | although he assumes a single groupby field, so your dict version makes more sense | 18:59 |
| gordc | dhellmann: did you just edit the wiki? | 18:59 |
| terriyu | speaking of which, what about documentation for the group by feature ? | 19:00 |
| dhellmann | gordc: no | 19:01 |
| *** eglynn-is-hungry has joined #openstack-metering | 19:01 | |
| dhellmann | terriyu: the autodoc code in sphinx will include the argument in our api docs | 19:01 |
| gordc | dhellmann: oh.. then i think you commented on it before: We probably want that to be a mapping between the field name and its value. {'metadata.instance_type': 'm1.tiny'} -- dhellmann | 19:01 |
| dhellmann | gordc: oh, yeah, that's probably old from when it was originally posted | 19:02 |
| gordc | seems like you already knew what terriyu was going to produce | 19:02 |
| dhellmann | gordc: the reason I have such a bad memory is I exist outside of your space-time continuum | 19:02 |
| gordc | terriyu: i guess that resolves that issue. | 19:02 |
| dhellmann | terriyu: although we should add some examples of using the API, with real detail about the results -- that can come in another patch, IMO | 19:03 |
| terriyu | dhellmann: exactly what I was thinking | 19:03 |
| dhellmann | terriyu: I posted one more comment about the new version | 19:04 |
| gordc | dhellmann: considering how much you cover in openstack alone, i'm shocked if you remember 5% of it. | 19:04 |
| dhellmann | many fingers in many pies, mmm, pie ;-) | 19:05 |
| terriyu | dhellmann: saw your comment. If validate_groupby_fields() removes duplicates, should it still be called validate_groupby_fields ? | 19:06 |
| gordc | dhellmann: mmmm... now i'm just going to be thinking of pie for next half hour. | 19:06 |
| dhellmann | terriyu: I think it's ok, unless you have a better name you'd like to use? | 19:07 |
| *** SergeyLukjanov has quit IRC | 19:07 | |
| * dhellmann goes to get cake from the fridge | 19:08 | |
| terriyu | dhellmann: no, I don't know of a better name. Just checking, since I'm no expert. | 19:08 |
| dhellmann | terriyu: leave it as validate, then, and we'll see if someone else suggests something :-) | 19:09 |
| terriyu | dhellmann: sounds good :) So, the rest of the code looks okay? | 19:09 |
| dhellmann | terriyu: yes, nice and clean | 19:10 |
| terriyu | great | 19:10 |
| dhellmann | terriyu: oh, a couple of extra unit tests for validate_groupby_fields would be good -- I suggested a list, I think they'll be pretty straightforward to add | 19:10 |
| *** dina_belova has joined #openstack-metering | 19:11 | |
| terriyu | dhellmann: sounds good. gordc was writing some similar tests for the more complicated _query_to_kwargs() -- the method that processes the query part of the request | 19:13 |
| terriyu | https://review.openstack.org/#/c/43796/ | 19:13 |
| * dhellmann reads | 19:14 | |
| gordc | terriyu: did you want me to rebase off your patch and add testcases for you? | 19:14 |
| terriyu | gordc: I have no idea what I'm doing, so I'll do whatever you and dhellmann think is best :) | 19:15 |
| gordc | probably best to contain it in your patch but whatever works. | 19:15 |
| dhellmann | let's try to get the tests into the changeset that adds the function being tested this time | 19:15 |
| terriyu | I can try writing some tests | 19:16 |
| *** dina_belova has quit IRC | 19:16 | |
| gordc | terriyu: you can do something similar to mine... should be even simpler. | 19:17 |
| dhellmann | gordc: in https://review.openstack.org/#/c/43796/5/tests/api/v2/test_query.py why is filtering on timestamp invalid? | 19:18 |
| dhellmann | oh, nevermind, that's an invalid field isn't it | 19:18 |
| dhellmann | it's either start_timestamp or end_timestamp | 19:18 |
| terriyu | dhellmann: to remove the duplicate fields in _validate_groupby_fields(), would it be okay to do something like list(set(groupby_fields)) | 19:19 |
| dhellmann | terriyu: yes, that's exactly the right thing to do | 19:19 |
| terriyu | excellent | 19:19 |
| gordc | dhellmann: the last test? it's because we only support querying on a range. not an exact timestamp | 19:20 |
| dhellmann | oh, wait, unless we care about the order of those fields for some reason? | 19:20 |
| dhellmann | gordc: do we support querying against metadata with operators other than =? | 19:20 |
| dhellmann | terriyu: do we care about the order for the group by fields? | 19:21 |
| terriyu | dhellmann: not that I'm aware of | 19:21 |
| gordc | dhellmann: nope. based on how i read it, it only supports filtering on a range for timestamp and only = for everything else. | 19:22 |
| dhellmann | gordc: :-( | 19:22 |
| dhellmann | ok | 19:22 |
| dhellmann | I guess that's not new | 19:22 |
| gordc | dhellmann: yeah, probably worth looking at a resuable way to handle both. | 19:23 |
| dhellmann | yeah, we can do that in icehouse | 19:23 |
| terriyu | so for now, list(set(groupby_fields)) is okay? | 19:23 |
| dhellmann | terriyu: yes | 19:23 |
| terriyu | ok | 19:23 |
| dhellmann | if someone points out we need to retain the order, we can do something different | 19:23 |
| terriyu | gordc, dhellmann : where should I put the unit tests for _validate_groupby_fields() ? In ceilometer/tests/api/v2/test_query.py ? | 19:28 |
| dhellmann | yeah, a new test class in that file seems OK | 19:29 |
| gordc | terriyu: seems fine to me as well. | 19:29 |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: 'and' and 'or' operations for alarms combination https://review.openstack.org/42832 | 19:29 |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: Add alarm combination API https://review.openstack.org/41971 | 19:29 |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: Move alarm defaults to the API https://review.openstack.org/45085 | 19:29 |
| *** yjiang5 is now known as yjiang5_away | 19:33 | |
| sandywalsh_ | sileht: dhellmann, not sure if you saw the json filter I wrote for the nova scheduler? It could be a common/easy way to do the and/or/in/not stuff needed by statistics and alarming. | 19:41 |
| sandywalsh_ | https://github.com/openstack/nova/blob/master/nova/scheduler/filters/json_filter.py | 19:41 |
| sandywalsh_ | you can do some pretty complex scheduler requests with it | 19:42 |
| sandywalsh_ | might be interesting to refactor it into oslo and share it across all these projects | 19:42 |
| sandywalsh_ | basically you pass in your complex query as a json body in the http request | 19:42 |
| dhellmann | sandywalsh_: would it be useful for building a query to pass to the database? | 19:44 |
| dhellmann | it looks like it operates on items in memory? | 19:44 |
| sandywalsh_ | it does ... I'll have a look at the query -> db query code and see how it might map. | 19:45 |
| sandywalsh_ | I think a Visitor pattern on it would helkp | 19:45 |
| sandywalsh_ | help | 19:45 |
| sandywalsh_ | and make it useful for anyone (just implement your own visitor and variable lookup utility) | 19:45 |
| sandywalsh_ | not sure how "optimal" it would be | 19:46 |
| sandywalsh_ | likely, not very | 19:46 |
| dhellmann | still, we might get some ideas for the data structures we'd need | 19:48 |
| sandywalsh_ | yep. It's such an easy construct and yet very powerful (and works well with web clients) | 19:50 |
| *** dina_belova has joined #openstack-metering | 20:12 | |
| jd__ | terriyu: you rock, we may be able to finish your blueprint before ttx's coffee :) | 20:12 |
| terriyu | jd__: thanks :) I'm working on a new patch set, so let me know if you have any comments | 20:13 |
| jd__ | who's gonna win implement-a-blueprint-before-h3 contest | 20:13 |
| *** shaneduan is now known as shaneduan[afk] | 20:13 | |
| jd__ | terriyu: yeah I've just said I agree with dhellmann and that it could deserve a test | 20:13 |
| terriyu | jd__: dhellmann, gordc, and I also thought that instead of using g, we should use groupby | 20:13 |
| terriyu | like: http://localhost:8777/v2/meters/instance/statistics?groupby=user_id | 20:14 |
| terriyu | jd__: what do you think? | 20:14 |
| jd__ | I clicked on this link | 20:14 |
| * jd__ facepalm | 20:14 | |
| gordc | jd__: i did that the first time terriyu did it. | 20:15 |
| jd__ | terriyu: oh right I missed that, yes totally groupby is much better | 20:15 |
| jd__ | good thing you raised it | 20:15 |
| *** dina_belova has quit IRC | 20:17 | |
| openstackgerrit | A change was merged to openstack/ceilometer: Add query support to alarm history API https://review.openstack.org/44908 | 20:19 |
| jd__ | brb | 20:21 |
| openstackgerrit | gordon chung proposed a change to openstack/ceilometer: add tests for _query_to_kwargs func https://review.openstack.org/43796 | 20:36 |
| openstackgerrit | Monty Taylor proposed a change to openstack/ceilometer: Update to tox 1.6 and setup.py develop https://review.openstack.org/45124 | 20:49 |
| *** evanjfraser has joined #openstack-metering | 20:54 | |
| openstackgerrit | Terri Yu proposed a change to openstack/ceilometer: Add group by statistics in API v2 https://review.openstack.org/44130 | 21:06 |
| openstackgerrit | Mehdi Abaakouk proposed a change to openstack/ceilometer: Disable the pymongo pooling feature for tests https://review.openstack.org/44465 | 21:10 |
| *** dina_belova has joined #openstack-metering | 21:12 | |
| *** dina_belova has quit IRC | 21:17 | |
| *** shaneduan[afk] is now known as shaneduan | 21:30 | |
| openstackgerrit | gordon chung proposed a change to stackforge/pycadf: bump oslo.config req to 1.2.0a3 https://review.openstack.org/45128 | 21:31 |
| *** shaneduan is now known as shaneduan[afk] | 21:32 | |
| thomasm | gordc, Thanks for your comments. I definitely agree. I want to be sure I don't miss the mark for getting this into H3, so how does the deadlines work for that? | 21:35 |
| thomasm | s/does/do/ | 21:35 |
| gordc | thomasm: https://wiki.openstack.org/wiki/Havana_Release_Schedule i don't know what time it is in UTC but i assume cut off is soon. | 21:40 |
| *** osphy has joined #openstack-metering | 21:40 | |
| gordc | yours is a bug fix so you have a bit longer. | 21:40 |
| thomasm | gordc, Ohhhh okay | 21:40 |
| thomasm | There we go. Thanks. =] | 21:40 |
| *** thomasm has quit IRC | 21:43 | |
| *** shaneduan[afk] is now known as shaneduan | 21:50 | |
| openstackgerrit | A change was merged to stackforge/pycadf: bump oslo.config req to 1.2.0a3 https://review.openstack.org/45128 | 21:52 |
| *** shardy is now known as shardy_afk | 21:53 | |
| *** sdake_ has quit IRC | 21:54 | |
| *** dina_belova has joined #openstack-metering | 22:13 | |
| *** dina_belova has quit IRC | 22:18 | |
| *** boris-42 has quit IRC | 22:19 | |
| *** boris-42 has joined #openstack-metering | 22:19 | |
| openstackgerrit | Eoghan Glynn proposed a change to openstack/ceilometer: Clean-ups related to alarm history patches. https://review.openstack.org/45135 | 22:24 |
| openstackgerrit | A change was merged to openstack/ceilometer: Add group by statistics in API v2 https://review.openstack.org/44130 | 22:25 |
| eglynn-is-hungry | sandywalsh_, dhellmann: your non-blocking comments on alarm history patches addressed in https://review.openstack.org/45135 | 22:27 |
| *** changbl has quit IRC | 22:27 | |
| *** eglynn-is-hungry is now known as eglynn | 22:28 | |
| *** eglynn is now known as eglynn-is-tired | 22:28 | |
| openstackgerrit | Eoghan Glynn proposed a change to openstack/ceilometer: Clean-ups related to alarm history patches https://review.openstack.org/45135 | 22:34 |
| *** dhellmann is now known as dhellmann_ | 22:43 | |
| openstackgerrit | Eoghan Glynn proposed a change to openstack/ceilometer: Clean-ups related to alarm history patches https://review.openstack.org/45135 | 22:44 |
| *** shakayumi has joined #openstack-metering | 22:44 | |
| openstackgerrit | Eoghan Glynn proposed a change to openstack/ceilometer: Clean-ups related to alarm history patches https://review.openstack.org/45135 | 22:45 |
| *** gordc has quit IRC | 22:54 | |
| *** eglynn-is-tired has quit IRC | 22:55 | |
| *** sdake_ has joined #openstack-metering | 22:56 | |
| *** dina_belova has joined #openstack-metering | 23:13 | |
| *** Ruetobas has quit IRC | 23:14 | |
| *** dina_belova has quit IRC | 23:18 | |
| *** Ruetobas has joined #openstack-metering | 23:19 | |
| *** shaneduan is now known as shaneduan[afk] | 23:22 | |
| *** shaneduan[afk] is now known as shaneduan | 23:23 | |
| *** sdake_ has quit IRC | 23:24 | |
| openstackgerrit | Monsyne Dragon proposed a change to openstack/ceilometer: Add configuration-driven conversion to Events https://review.openstack.org/42713 | 23:53 |
| *** eglynn-is-tired has joined #openstack-metering | 23:56 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!