[09:09] belated good morning, all :) [09:36] hmm [09:37] facubatista: playing with charmcraft master+43+46, with --verbose, i think we need a couple more log lines :) [09:37] i'll propose something in a bit [09:58] facubatista: boo, it's more work than i wanted it to be; filed charmcraft#48 and charmcraft#49 so we don't forget [09:58] Issue charmcraft#48: initialization order means early log lines can be lost [09:58] Issue charmcraft#49: charmcraft should log where it's logging to, and when the logfile is removed [10:00] facubatista: unrelatedly, i'm concerned the store ops are slow. I'll be asking about this in the meeting today... [10:05] facubatista: OTOH 'slow' is 50ms from it being a snap, 150ms from all the extra imports for store things, 250ms from the actual store request [10:05] but it does but it at 500ms which is definitely slow [10:06] *put it [10:19] * Chipaca breaks a take [11:02] Muy buenos días a todos! [11:10] facubatista: [citation needed] :-p [11:10] facubatista: hola :) [11:10] hola :) [11:13] facubatista: i'm concerned that 'charmcraft whoami' easily takes over 500ms [11:14] facubatista: just fyi; it's not actionable yet [11:14] mostly raising a bit of stink over in https://github.com/go-macaroon-bakery/py-macaroon-bakery/issues/76 [11:14] Chipaca, I can move the httbakery/etc imports to when the first time any store related thing is used [11:15] facubatista: that won't stop whoami taking over 500ms :) [11:15] ah, right, jaja [11:15] at some point, yes, we want to make things import just what's needed [11:15] but not yet, not this early [11:15] Chipaca, I don't like the 500ms either [11:16] facubatista: at least part of it is the server being slow, fwiw [11:16] facubatista: I'm *also* going to raise that [11:16] (in its defense, it is staging) [11:17] /v2/whoami takes between 100 and 250ms [11:17] *both* of those are two big [11:17] too* [11:17] 10 times too big in fact :) [11:17] but, we'll see [11:17] Chipaca, that is when credentials are valid, right? [11:17] yes [11:17] ack [11:18] i haven't even looked at slow paths here :) [11:18] anyway [11:18] back to reviewing john's code [11:24] Chipaca, https://github.com/canonical/charmcraft/issues/50 [11:25] Chipaca, what do you feel about renaming `_save_credentials` to `_check_save_credentials`? [11:25] _save_credentiarola [11:27] facubatista: _save_credentials_if_changed ? [11:27] Chipaca, facubatista /wave [11:27] jam: 👋 [11:27] hola jam [11:27] I'm certainly flexible on the naming, and we can stick with save, but it did feel a bit like one of those places that you would start digging in "why didn't it get updated", etc. [11:30] yeah, i agree with that :) [11:30] facubatista: do you see this method growing any other checks before actually saving? [11:32] "_maybe_save_credentials" [11:32] _save_credentials_or_dont_yolo [11:32] :-) [11:33] I like ensure for that sort of thing, but it doesn't feel great here [11:34] i need to get lunch done, bbiab [11:35] Chipaca, I don't think it would also test other thing; its backend may change (file -> wallet), but it will essentially remain like this [11:43] facubatista: then i'll stick with my suggestion above [11:44] facubatista: _save_credentials_if_changed ? ← this one i mean [11:45] Chipaca, ok [11:49] * facubatista likes _save_credentials_if_changed because it's exactly what the docstring says [11:53] Chipaca, jam, all comments addressed; also annotated open issues to correspondant XXXs [11:53] and pushed! [12:28] jam: i'm canceling the standup fyi [12:54] facubatista: WRT building the snap to edge, yes but version should probably be generated from git for that [12:55] facubatista: otherwise everything on edge will always have which is a lie :) [12:55] facubatista: would you mind if we did the same thing we do in ops? [13:05] Chipaca, +1 [13:12] facubatista: post-release tho [13:22] ack [13:24] d'oh, i merged something i should not have [13:24] * Chipaca ← bad person [13:32] Chipaca, btw, missing your review here: https://github.com/canonical/charmcraft/pull/43 [13:32] PR charmcraft#43: Store authentication related commands [15:05] facubatista: https://github.com/canonical/charmcraft/pull/54 plz [15:05] PR charmcraft#54: address issues raised in review of #46 that i merged too fast [15:56] facubatista: is charmcraft#43 needing anything more from your end? [15:56] PR charmcraft#43: Store authentication related commands [16:22] PR operator#337 opened: trivial change to test something [16:24] Chipaca, I need travis to end [16:24] ja, I summoned the response (?) [16:24] Chipaca, landed! [16:24] aww [16:25] rtd could give us docs for pull requests [16:25] but i need to be able to authorize the organization [16:31] PR operator#337 closed: trivial change to test something [19:11] 2020-06-24 16:11:10,768 charmcraft.commands.build DEBUG :: ERROR: Cannot find command 'git' - do you have 'git' installed and in your PATH? [19:12] Chipaca, maybe we need to put git and other VCs [19:12] *VCSs tools inside the snap? [20:34] facubatista: how did you get that one? [20:34] facubatista: (and yes i was expecting to have to do that, it's fine) [20:34] facubatista: (just not yet; i hadn't notied we were using git yet -- where are we?) [20:35] hm [20:35] i guess if you have a git url in your requires? [21:10] Chipaca, xactly [21:10] drat :) [21:10] facubatista: something you were trying, or a user reported this? [21:11] Chipaca, a user reported something similar, I'm not sure if exactly the same thing (need to see the logs they passed later) [21:11] Chipaca, we need also to "see" which `pip` they have [21:11] facubatista: we're shipping pip in the snap already [21:11] that much i did test :) [21:12] Chipaca, yes, but what if you run from tarball or whatever? seeing that the user has a different pip from "us" helps to understand a possible different behaviour [21:13] even from snap: we may have different pips in edge than stable, etc, so have that explicitly in the logs I think it worths having an extra system call when building [21:14] facubatista: you can start the build with 'pip list' [21:15] oh, that's probably better than `pip --version` [21:15] i think so :) [21:16] great [21:19] facubatista: hadn't the change to create the tempdir landed? [21:20] maybe i misremember [21:36] Chipaca, the tempdir? nop, I don't think we did that [21:36] Chipaca, I landed the creation of the config directory, for the credentials [21:41] Chipaca, `list` gives much juicy information that just the pip version [21:41] facubatista: ok i'll include creating the tempdir in my snap changes [21:42] facubatista: yes, useful information [21:42] facubatista: look at what travis prints out for example [21:42] facubatista: that's a good war log [21:49] Chipaca, having a simple "pip list" first is an awesome way to see if there's *any* problem with pip, before the quite more complex install itself [21:50] Chipaca, https://github.com/canonical/charmcraft/pull/56 [21:50] PR charmcraft#56: Make pip list current packages before proceeding with the install [21:56] Chipaca, remember we removed the `--system` parameter in the install call? I think the lack of it is creating the failure here: https://github.com/canonical/charmcraft/issues/55 [21:59] facubatista: let me ask one other thing there [21:59] Chipaca, I would let you ask even three things [21:59] qué compromiso /o\ [22:00] it's a max, not a hard number :p [22:00] like when the genius gives you UP TO three desires [22:01] wishes* [22:01] genie* [22:01] first wish: infinite genies [22:03] facubatista: strange that in spanish genie (djinn) and genius, are the same word (very different etymologies afaict) [22:04] facubatista: ('genio' in the djinn sense is not from latin) [22:04] "wishes": I should have known; "genie": didn't know [22:05] "Según parece, la palabra española «genio» proviene del árabe «djinn» cuyo significado describe a un tipo muy preciso de ser." [22:06] ah, genius comes from Latin [22:07] facubatista: as does genio in the genius sense :) [22:07] ah, "genio" from djinn is about "mood"? [22:08] facubatista: hmm... i hadn't thought about that, but probably [22:08] that's a weird one :) [22:08] facubatista: another one that spanish mashes together is scatology and escatology [22:08] but that one i love :-D [22:09] oh, that one is awesome [22:10] it totally mutates the meaning of "chistes escatológicos" [22:12] facubatista: good news on that bug [22:14] Chipaca, yes, but I suspected, "from snap" would be the main method of running snap, that's why I wanted for *us* to be always testing really-really-edge [22:14] s/running snap/running charmcraft/ [22:14] facubatista: yep [22:14] facubatista: also we'll need spread tests for the snap soon [22:15] anyway, tagged, assigned, and milestoned [22:15] and unblocked the user [22:15] rock [22:15] i declare this a great success [22:15] and go to bed [22:18] facubatista: 👋 eod for me [22:19] * facubatista eods too