20:00:12 <robcresswell> #startmeeting horizon
20:00:13 <openstack> Meeting started Wed Jul 27 20:00:12 2016 UTC and is due to finish in 60 minutes.  The chair is robcresswell. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:00:14 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
20:00:17 <robcresswell> o/
20:00:18 <openstack> The meeting name has been set to 'horizon'
20:00:21 <lcastell> o/
20:01:17 <matt-borland> o/
20:01:23 <tsufiev> o/
20:02:09 <robcresswell> Hey folks
20:02:28 <lcastell> hey
20:02:29 <robcresswell> We are without agenda items again
20:02:52 <robcresswell> So we can just move to open discussion if anyone has anything to raise
20:03:07 <matt-borland> PhantomJS?
20:03:11 <robcresswell> #topic Open Discussion
20:03:19 <matt-borland> the gate npm-run-test
20:03:24 <robcresswell> Ah, yeah
20:03:39 <robcresswell> Do e have a cause yet?
20:03:41 <robcresswell> we*
20:03:44 <matt-borland> I will happily help figure out what is going on, if I can get some help with how to debug
20:04:23 <matt-borland> I think I need to talk to krotscheck about how to do that.
20:04:28 <robcresswell> I would imagine digging up krotscheck blog on building a local node would be sensible
20:04:38 <robcresswell> See if it can be reproduced locally
20:04:51 <krotscheck> I hear that krotscheck has a blog whose URL is suprisingly similar to his IRC handle
20:05:00 <matt-borland> heh
20:05:04 <tsufiev> lol )
20:05:07 <david-lyle> https://krotscheck.net/2016/06/01/how-to-simulate-an-openstack-infra-slave.html
20:05:14 <matt-borland> thanks :)
20:05:26 <david-lyle> I seem to have that bookmarked :)
20:05:46 <matt-borland> In other news, not quite related, do devs prefer to have tests run in PhantomJS or in Chrome/FF?
20:05:54 <matt-borland> I had a complaint about having to use Chrome.
20:06:00 <matt-borland> *heard a complaint
20:06:05 <robcresswell> I don't have any strong preference
20:06:09 <matt-borland> ok.
20:06:13 <robcresswell> It seems to bug some people that a window opens temporarily
20:06:17 <robcresswell> in chrome
20:06:46 <matt-borland> yeah.  Maybe we can make it configurable (without changing the code itself)
20:06:56 <matt-borland> right now you have to edit karma.conf to modify that
20:07:07 <robcresswell> the gate is the bigger issue :)
20:07:11 <tsufiev> I recall PhantomJS had some issues with file inputs
20:07:12 <matt-borland> yep
20:07:20 <robcresswell> tsufiev: Yep
20:07:24 <tsufiev> though I do not know if it still does
20:07:33 <matt-borland> Phantom doesn't respect things like Promise/File objects right off the bat
20:07:46 <matt-borland> some of those presumed globals
20:08:22 <matt-borland> anyway, ok, so fixing the npm-run-test (and -lint!) is a priority, cool
20:08:34 <matt-borland> I'll budget quiet time for that tomorrow
20:08:36 <david-lyle> what's the upside to testing in a non-realworld browser?
20:09:26 <robcresswell> *silence*
20:09:31 <matt-borland> not much, mostly that it doesn't pop up a window, or require Chrome (which is not open source, yadda yadda)
20:09:53 <robcresswell> it can be a good deal faster too.
20:09:54 <tsufiev> also we can use xvfb to hide browser window
20:10:05 <robcresswell> Which is the important part :p
20:10:12 <tsufiev> if that's the real problem
20:10:25 <matt-borland> that's true, there's slightly less runup for PhantomJS
20:10:32 <matt-borland> doesn't sound like a big priority.
20:10:43 <matt-borland> that's fine with me, just asking the question.
20:10:43 <robcresswell> I don't consider a window popping up an issue, ha. Lets just debug the gate timeouts.
20:10:47 <matt-borland> yep
20:11:32 <robcresswell> N-3 is 4 weeks tomorrow
20:11:41 <david-lyle> :o
20:11:48 <robcresswell> So make sure to be wrapping up features etc
20:12:10 <matt-borland> I think with tsufiev's addition of basic integration tests, we have only one more known hurdle to make Angular Images default.
20:12:32 <tsufiev> looking at the patch in browser right now
20:12:36 <tsufiev> I mean how it works
20:12:44 <matt-borland> The other hurdle is displaying 'in transition' statuses for the row/field in a list.
20:12:55 <matt-borland> Matt Wood has a patch for that.
20:13:05 <robcresswell> Does that solve the issue of running both sets of tests/
20:13:05 <tsufiev> I have a slight concern that although we have an integration 'smoke' test for angular content, we don't run it
20:13:10 <robcresswell> ?*
20:13:12 <matt-borland> so I'd like that all to land within about 5 days
20:13:27 <tsufiev> so even if something will break NG Images panel, we won't notice that until run the test explicitly
20:13:59 <robcresswell> I didnt think we had a solution for running both at once yet
20:14:30 <robcresswell> probably just needs a separate job, but need to figure out the best way of toggling that
20:14:36 <tsufiev> I had an idea of some tricky setup to pass an alternate Django setting using cookies in integration tests
20:14:45 <robcresswell> :/ cookies?
20:14:46 <tsufiev> didn't investigate yet
20:14:55 <matt-borland> can we not alter settings as part of the test run?
20:15:05 <matt-borland> (an independent test run?)
20:15:15 <tsufiev> not, because integration test is a separate process from horizon webserver
20:15:27 <tsufiev> we would have to develop our own way of IPC
20:15:34 <matt-borland> but I mean, we can modify that config file too...right?
20:15:51 <tsufiev> or... I have an idea
20:15:53 <matt-borland> isn't that why there's a horizon.config for the unit tests?
20:16:00 <matt-borland> er, integration tests?
20:16:03 <tsufiev> we could create a new dsvm job for angular content :)
20:16:09 <tsufiev> less troubles with cookies
20:16:10 <robcresswell> I'd have thought the easiest way was just to append a boolean to the conf file in the pre test hook
20:16:19 <robcresswell> or something like that
20:16:32 <robcresswell> tsufiev: Yes, this was my thinking. Separate job seems cleanest
20:16:36 <tsufiev> robcresswell, but it still will be either of these 2 values in a single test run
20:16:50 <robcresswell> either is fine, as we can just run two jobs
20:17:01 <robcresswell> Having it swap halfway is hacky
20:17:06 <matt-borland> I'm assuming most of the time for the integration is the dsvm setup?
20:17:08 <tsufiev> matt-borland, horizon.conf is a way to tell integration tests about horizon enabled features
20:17:17 <matt-borland> yep, exactly tsufiev
20:17:26 <tsufiev> horizon.conf and openstack_dashboard.settings may be theoretically disjoint
20:17:34 <tsufiev> er, inconsistent
20:17:38 <matt-borland> of course
20:18:11 <tsufiev> okay, so a separate job for angular content, that sounds pretty reasonable to me
20:18:19 <tsufiev> I hope infra folks won't blame us :D
20:18:38 <matt-borland> If they do, we cross that bridge.  I'm glad we at least have tests that respond to current settings.
20:18:56 <matt-borland> and operate in both panel enabled situations (even if separately)
20:19:06 <matt-borland> thanks again for your help with that tsufiev
20:19:35 <tsufiev> if we hit the topic of different Horizon modes & integration tests, I have a related question
20:19:43 <matt-borland> so, to be clear, we can approve the switch patch before that's worked out, we just don't want to throw the switch itself to Angular, right?
20:19:44 <robcresswell> Yes, thats a step in the right direction
20:19:51 <tsufiev> I have a patch for [Django] Create Image which uses CORS
20:19:57 <robcresswell> Yes, matt-borland
20:20:02 <matt-borland> thanks
20:20:07 <tsufiev> but a CORS support is a separate feature
20:20:17 <robcresswell> We need both being tested in the gate before we can swap the default.
20:20:27 <tsufiev> don't have the luxury of creating another job just to test it alone :)
20:20:30 <robcresswell> but nothing to stop allowing deployers to swap
20:20:43 <matt-borland> yep, sounds great robcresswell
20:20:57 <tsufiev> it's the same issue as with testing both angular and legacy content
20:21:06 <robcresswell> tsufiev: Even with a separate job, how would the ocnfiguration be job-specific?
20:21:23 <tsufiev> another setting switch in a pre-test hook
20:22:07 <robcresswell> Right. Sorry if I'm being slow, but how is the pre-test hook job specific? I thought it lived within horizon
20:22:40 <tsufiev> well, we'll need to provide at least two pre-test hooks, one for legacy (existing), another for legacy
20:22:47 <robcresswell> Or is the implication that the job called "angular pre-test hook" or similar
20:22:52 <robcresswell> ah, yep
20:23:00 <tyr_> o/
20:23:03 <tsufiev> *another for angular
20:23:44 <robcresswell> tsufiev: If its just a a shell script, could we not pass in a parameter?
20:23:51 <tsufiev> matt-borland, I'm in for approving that patch even before we have a job separation
20:23:55 <robcresswell> Would be nice to avoid config script duplication
20:24:02 <tsufiev> robcresswell, I think we could
20:24:09 <tsufiev> 80% sure
20:24:12 <robcresswell> I think that would be cleaner.
20:24:37 <robcresswell> I'll have a look tomorrow. Something like ./pre_test_hook.sh ng would be convenient.
20:24:56 <matt-borland> robcresswell, keep me in the loop on those developments, I'd like to know how it all works :)
20:25:33 <matt-borland> and obviously if there's anything I can do to help get those jobs testing the way we want
20:26:10 <tsufiev> matt-borland, have your read docs on integration testing ;)?
20:26:18 <robcresswell> I'll discuss in infra tomorrow, see if they have any suggestions
20:26:39 <matt-borland> about how to set up pre-test-hooks, etc.? no
20:26:53 <matt-borland> tsufiev ^^ that is
20:27:03 <tsufiev> nope, about page object pattern and the importance of it :)
20:27:13 <matt-borland> I'm fine with it
20:27:20 <robcresswell> re: schema form, I've updated the whole thing to use the service patterns that are common now. The only bit thats got me stuck is returning data from the create service back to the LI modal. It creates the network fine, but it doesnt seem to pass the data how I expect it to.
20:27:31 <matt-borland> I will only complain if it doesn't work or is a problem, tsufiev
20:27:57 <tyr_> robcresswell: if you post an updated patch version I'm happy to help troubleshoot
20:28:28 <tsufiev> matt-borland, it may seem that it makes things slightly more complicated than they are, but in a long run it helps to keep the tests maintainable
20:28:34 <robcresswell> tyr_: Yeah, I will do that I think. Faster that way.
20:28:43 <robcresswell> tyr_: I'll ping about it tomorrow. Kinda spent today.
20:28:44 <matt-borland> tsufiev, I have not expressed complaints about pageobject
20:29:01 <matt-borland> it's similar to other testing infrastructures I've used, it's fine
20:29:22 <tsufiev> matt-borland, sorry then, I have misinterpreted your message
20:29:42 * matt-borland means 'fine' when he says 'fine,' not 'FINE!' :)
20:29:49 <tsufiev> :)
20:30:29 <matt-borland> tsufiev, I had just taken a very simple and admittedly sloppy approach to get started
20:31:46 <tsufiev> np, I prefer to get things done fast myself
20:32:03 <robcresswell> I've been starring patches that I'd like to see make the release in the next month
20:32:14 <robcresswell> #link https://review.openstack.org/#/q/starredby:rob-cresswell+status:open
20:32:51 <tsufiev> I expected to see at least 50 patches there
20:33:13 <robcresswell> ha, I've just started doing it recently. The idea is to use that to advertise patches-to-review
20:33:59 <robcresswell> For the most part, file upload, angular images, filtering improvements and django 1.10 compatibility
20:34:06 <matt-borland> I like the idea robcresswell, thanks
20:34:19 <ediardo> got it
20:34:42 <robcresswell> If you want patches to focus reviews on, this list aims to be the critical ones.
20:35:22 <tsufiev> yep, it's much more maintainable than a wiki page, robcresswell
20:35:37 <robcresswell> tsufiev: Yes, much lower input!
20:36:17 <robcresswell> In case anyone else didnt realise, you can customise your gerrit at Settings > Preferences and add custom links
20:36:41 <robcresswell> Really useful to put davids review dashboard in there as well as that starred link. Helps focus reviews on important items better.
20:37:02 <robcresswell> Thats all from me; any other issues? Otherwise we shall end early
20:37:57 <tsufiev> I'd like to ask only about context['THEME'] fix
20:38:40 <tsufiev> folks from murano team asked about it several times and every time I had to point them to a zhurong's patch which does it wrong :/
20:38:42 <hurgleburgler> isn't it a configuration problem?
20:38:44 <robcresswell> Are there any plugins still blocked on that? I had thought the config updates resolved that issue?
20:39:36 <tsufiev> I don't understand either how they managed to get this error
20:39:39 <tyr_> I see it anytime I'm running COMPRESS_ENABLED=True.
20:39:51 <tyr_> What is the current idea for the fix?
20:40:00 <tsufiev> https://review.openstack.org/#/c/344871/ tyr_
20:40:13 <hurgleburgler> I run compress enabled true all the time
20:40:47 <hurgleburgler> the thing is … the fix is just a bandaid
20:40:52 <tsufiev> I guess it depends on some missing config value
20:40:56 <hurgleburgler> the context sets the default using the default value its supposed to
20:40:58 <hurgleburgler> using the settings
20:41:07 <tyr_> I do not have COMPRESS_OFFLINE=True fwiw
20:41:18 <tsufiev> which... tells me that a few unit tests for themes wouldn't hurt, hurgleburgler ;)
20:41:22 <hurgleburgler> if the settings aren't set properly, then it can't know what to default to
20:41:53 <tyr_> which settings in particular hurgleburgler?
20:42:01 <robcresswell> To be clear, this fails *only* in a unit test environment
20:42:03 <hurgleburgler> tsufiev: there was talk last year of a need for unit tests for the static settings in general
20:42:11 <robcresswell> and *only* when you do not import from Horizon
20:42:29 <hurgleburgler> DEFAULT_THEME and AVAILABLE_THEMES
20:42:48 <hurgleburgler> you really only need AVAILABLE_THEMES, DEFAULT_THEME will fall back to AVAILABLE_THEMES[0]
20:43:00 <tyr_> both are set in my settings.py, yet I see the THEME compress error
20:43:03 <hurgleburgler> and if you don't have AVAILABLE_THEMES set, it falls back to the previous setting, custom_theme_path
20:43:06 <robcresswell> https://review.openstack.org/#/c/344871 updated. Still a bandaid.
20:43:59 <hurgleburgler> ugh, that fix duplicates logic that already exists else where
20:44:10 <hurgleburgler> so now we would be doing the exact same default check 2-3 times
20:44:24 <robcresswell> Wait, where else does it do that check?
20:44:32 <robcresswell> the issue just arises because that function defaults to []
20:44:50 <robcresswell> if it defaults to the default theme, it should work fine, I believe.
20:44:56 <hurgleburgler> you are assuming that 'default' exists
20:44:57 <hurgleburgler> it migh tnot
20:45:07 <hurgleburgler> someone might not be configured for 'default' at all
20:45:10 <tsufiev> commented on that inline
20:45:12 <tsufiev> hm
20:45:40 <tsufiev> hurgleburgler, is there a way to enforce at least one theme to be available?
20:45:43 <hurgleburgler> that's why there is a DEFAULT_THEME setting
20:45:55 <hurgleburgler> it should already be enforcing that … but I can take a look again
20:46:35 <robcresswell> if you try and run horizon without that key it will enforce it and fall over immediately
20:46:51 <robcresswell> from my tests the other day
20:47:30 <robcresswell> Either way, at some point in the chain its not throwing an error or defaulting as desired
20:47:54 <robcresswell> Also, rather unfortunately, the release nots at the time mention the deprecated settings but not the new required ones I believe.
20:48:10 * tsufiev sees 'enforcing' as failing and telling how to prevent a failure
20:48:13 <hurgleburgler> I'll take a look at it
20:48:37 <robcresswell> thanks hurgleburgler
20:48:46 <robcresswell> much appreciated :)
20:48:53 <tsufiev> hurgleburgler, thank you!
20:49:55 <hurgleburgler> np
20:51:56 <robcresswell> Thanks for your time everyone!
20:52:02 <robcresswell> #endmeeting