cjwatsontsimonq2: you have review comments01:37
tsimonq2cjwatson: Many thanks.01:38
tsimonq2cjwatson: To be fair with the whole whitespace thing, it was a wild shot in the dark, I didn't remember how to do that particular element at all; so thanks.01:42
cjwatsonNot a problem01:44
tsimonq2cjwatson: So about using "==" over "is", I think Steve prefers the opposite, and that's already approved, so for the sake of consistency unless you feel strongly about it, I'll keep it consistent ;)02:05
tsimonq2cjwatson: Even though that part does get removed in newer commits, I think it's just worth a quick note. :)02:05
cjwatsontsimonq2: I don't feel that strongly about it, and looking at the enum documentation I see that it explicitly guarantees that object identity comparison will work, so I guess it's OK02:11
tsimonq2cjwatson: ok02:13
tsimonq2cjwatson: Question: would it be acceptable to do something like `git rev-parse --symbolic-full-name bionic@{upstream}` and then just strip the `/`s and just get the third item from that (in this case, just the string "origin"), or might that be dynamic enough for it not to work?02:26
tsimonq2cjwatson: Or do you know of a better way to extract that string besides just doing a .split("/")[2]?02:27
cjwatsontsimonq2: There may be a better way (git surely knows how to do it internally), but I couldn't find it.  .split("/")[2] should be OK for now.02:29
tsimonq2cjwatson: Alright.02:29
tsimonq2cjwatson: So I've hit a dead end when you have a minute... the following code is what replaced the string replacement code I had before, with all other parts staying the same:03:03
tsimonq2lp_git_repo = options.launchpad.git_repositories.getByPath(03:03
tsimonq2                      path=urlparse(remote).path.lstrip("/"))03:03
tsimonq2I confirmed that urlparse(remote).path.lstrip("/") is in fact a string and it's equal to "~tsimonq2/ubuntu-seeds/+git/lubuntu"03:04
tsimonq2However, I'm getting thrown a 400 from Launchpad, more specifically: lazr.restfulclient.errors.BadRequest: HTTP Error 400: Bad Request03:04
tsimonq2I guess I'm wondering if this is PEBKAC or if this is Launchpad.03:06
cjwatsonIn [1]: lp.git_repositories.getByPath(path="~tsimonq2/ubuntu-seeds/+git/lubuntu")03:16
cjwatsonOut[1]: <git_repository at https://api.launchpad.net/devel/~tsimonq2/ubuntu-seeds/+git/lubuntu>03:16
cjwatsonSo I'm not seeing the problem.  Is the code above where the exception is being raised?03:16
cjwatsonYou'd get a 404 from trying to set default_branch if there's no such branch in the repository yet as far as LP knows; if you've only just pushed it then beware that it takes a short but non-zero amount of time for LP to scan the repository, so you might need a retry/backoff loop to handle that.  Not sure where you'd get a 400 though.03:18
cjwatson(Or rather from trying to .lp_save() after setting .default_branch)03:19
tsimonq2Right, it does occur there, and that could certainly be a possibility because in the Python console it works fine if I take it step by step keeping variable names and values03:20
cjwatsonYou should print the content of the exception to find out more detail.03:20
tsimonq2Would this be verbose enough? https://paste.ubuntu.com/26401875/03:21
cjwatsonI'm not seeing why that would return 400.  getByPath should just return None if there's no match.03:23
tsimonq2Let me push my work in progress code real quick...03:24
cjwatsonTry putting "import httplib2; httplib2.debuglevel = 1" near the top of your code to dump the actual request that's happening.  (Beware that it will contain your OAuth secrets, so will need to be sanitised and not just pastebinned.)03:25
tsimonq2Sure, while I'm doing that, I pushed my code to the MP.03:25
cjwatsonIt should have   send: 'GET /devel/+git?path=%22%7Etsimonq2%2Fubuntu-seeds%2F%2Bgit%2Flubuntu%22&ws.op=getByPath HTTP/1.1\r\nHost: api.launchpad.net\r\n   in it03:25
cjwatson(remote_git_repository rather than remote_git_branch, pedantically.)03:26
cjwatsonI see no obvious problems right now ...03:27
tsimonq2Right: send: 'GET /devel/+git?path=%22%7Etsimonq2%2Fubuntu-seeds%2F%2Bgit%2Flubuntu%5Cn%22&ws.op=getByPath HTTP/1.1\r\nHost: api.launchpad.net\r03:29
tsimonq2Then the next line is: reply: 'HTTP/1.1 400 Bad Request\r\n'03:30
cjwatsonWhat's that %5Cn doing in the encoding?03:30
cjwatsonTrailing literal "\n"03:30
cjwatsonremote_git_branch (or remote_git_repository) should call .rstrip("\n") on the result of subprocess.check_output before returning it03:31
tsimonq2That was it.03:31
tsimonq2Cool. :)03:32
tsimonq2Thanks cjwatson03:32
cjwatsonNo problem03:32
cjwatsonremote_branch doesn't have this problem because .splitlines() deals with it03:32
tsimonq2Oh, right.03:32
tsimonq2cjwatson: Now the MP should be updated with my latest commits that (I think) address all of your comments.03:34
cjwatsonI have some remaining whitespace nits, but can fix those up after merging rather than making you do them.03:37
cjwatson+def remote_git_repository(source, srcbranch):03:37
cjwatson+ remote = remote_git_repository(dest, options.source_series)03:37
cjwatsonthat looks weird03:37
cjwatsonbut apparently correct, just surprising naming03:38
tsimonq2Yeah, I think in that specific part of it, I sorta ran out of ideas (and was hoping you'd suggest alternatives if you had fitting ones), but it works. :P03:39
cjwatsonnot for this branch, but do we actually want to rename the whole git repository on disk from series to series?  I know there's legacy stuff to handle, but I'd have expected the Lubuntu seed repository to just be called "lubuntu" on disk, not lubuntu.bionic etc.03:39
tsimonq2I did that for the sake of backwards compatibility for any bit of tooling that happens to depend on Lubuntu's bionic seed being at lubuntu.bionic03:40
tsimonq2Of course given assurance that this won't break anything, I can refactor that.03:40
cjwatsonthe only such thing I know of is for the published seed branches on people.c.c/~ubuntu-archive/seeds/03:41
cjwatsonfor branch-seeds' purposes I think there'd be no breakage03:41
tsimonq2Alright, that's fine by me then. Would you like me to propose a follow-up MP or would you just like to do it?03:42
cjwatsontsimonq2: the former :)03:46
cjwatsontsimonq2: I've merged your branch and applied a few follow-up tweaks - thanks!03:46
tsimonq2cjwatson: Awesome, thanks.03:46
cjwatsontsimonq2: Oh, it occurs that for symmetry maybe you should have an lp_git_repository helper, just like lp_branch.03:47
cjwatsonwhich is extremely similar in structure ...03:47
tsimonq2cjwatson: I think it wouldn't be used because lp_branch seems to only be called in the Bazaar section of the code.03:48
tsimonq2cjwatson: Er, maybe you're saying that I should move the similar code up to its own separate function?03:49
tsimonq2What would be the advantages to doing that verus keeping it how it is?03:50
cjwatsontsimonq2: It's just odd to have it factored out to a separate function for bzr but not for git.03:53
cjwatsonNo good reason for that not to be symmetric.03:54
tsimonq2Right, but I could either put the contents of that function in the Bazaar section of the code and remove the function, or add an additional Git function. I'm thinking the former of which is borderline redundancy, but it's up to you because you have commit access. :)03:55
cjwatsonI don't care very deeply either way.03:58
tsimonq2cjwatson: Before I go ahead and commit this, would something like this be acceptable for what we talked about earlier? http://paste.ubuntu.com/26401984/04:00
tsimonq2Because in order to do the VCS detection, you need to have the directory, so that would be a catch 22 if we made it VCS-dependent.04:01
tsimonq2(<3 Vim)04:14
-queuebot:#ubuntu-release- New: accepted kylin-display-switch [amd64] (bionic-proposed) [1.0.1-0ubuntu1]05:42
-queuebot:#ubuntu-release- New: accepted libpath-iter-perl [amd64] (bionic-proposed) [0.2-1]05:42
-queuebot:#ubuntu-release- New: accepted pgq-node [amd64] (bionic-proposed) [3.2.5-1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-devtools [i386] (bionic-proposed) [1.13.4-1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-rhandsontable [amd64] (bionic-proposed) [0.3.4+dfsg-1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-utf8 [amd64] (bionic-proposed) [1.1.3-1]05:42
-queuebot:#ubuntu-release- New: accepted kylin-display-switch [i386] (bionic-proposed) [1.0.1-0ubuntu1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-devtools [amd64] (bionic-proposed) [1.13.4-1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-shinydashboard [amd64] (bionic-proposed) [0.6.1-1]05:42
-queuebot:#ubuntu-release- New: accepted papirus-icon-theme [amd64] (bionic-proposed) [20171223-1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-utf8 [i386] (bionic-proposed) [1.1.3-1]05:42
-queuebot:#ubuntu-release- New: accepted r-cran-psych [amd64] (bionic-proposed) [1.7.8-1]05:42
-queuebot:#ubuntu-release- New binary: r-cran-pillar [amd64] (bionic-proposed/universe) [1.0.1-1] (no packageset)06:19
-queuebot:#ubuntu-release- New binary: r-cran-broom [amd64] (bionic-proposed/universe) [0.4.3-1] (no packageset)06:20
=== s8321414_ is now known as s8321414
=== cpaelzer_ is now known as cpaelzer
cjwatsontsimonq2: "%s" % collection could just be spelled collection, but08:28
cjwatsontsimonq2: ... seems reasonable08:28
tsimonq2cjwatson: ack, thanks12:04
-queuebot:#ubuntu-release- Unapproved: grub2 (bionic-proposed/main) [2.02-2ubuntu3 => 2.02-2ubuntu3] (core)16:08
-queuebot:#ubuntu-release- Unapproved: accepted google-apputils-python [source] (xenial-proposed) [0.4.1-1ubuntu2.16.04.1]16:43
-queuebot:#ubuntu-release- Unapproved: accepted google-apputils-python [source] (artful-proposed) [0.4.1-1ubuntu2.17.10.1]16:44
cyphermoxcould someone please reject grub2 and grub2-signed from artful unapproved queue?16:54
-queuebot:#ubuntu-release- Unapproved: rejected grub2-signed [source] (artful-proposed) [1.85.1]17:07
-queuebot:#ubuntu-release- Unapproved: rejected grub2 [source] (artful-proposed) [2.02~beta3-4ubuntu8]17:07
-queuebot:#ubuntu-release- New binary: liburcu [i386] (bionic-proposed/main) [0.10.0-3] (ubuntu-desktop, ubuntu-server)17:15
-queuebot:#ubuntu-release- New binary: liburcu [amd64] (bionic-proposed/main) [0.10.0-3] (ubuntu-desktop, ubuntu-server)17:18
-queuebot:#ubuntu-release- Unapproved: accepted python-oslo.messaging [source] (artful-proposed) [5.30.0-0ubuntu2]17:19
-queuebot:#ubuntu-release- Unapproved: shim-signed (artful-proposed/main) [1.32 => 1.33.1~17.10.1] (core)20:30
-queuebot:#ubuntu-release- Unapproved: grub2 (artful-proposed/main) [2.02~beta3-4ubuntu7 => 2.02~beta3-4ubuntu7.1] (core)20:32
-queuebot:#ubuntu-release- Unapproved: grub2-signed (artful-proposed/main) [1.85 => 1.85.1] (core)20:33

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!