[02:03] <blr> wgrant: do you see anything obviously wrong with the validation or call to getByPath on l.829? https://code.launchpad.net/~blr/launchpad/ui-project-setbranch/+merge/259069
[02:04] <blr> wgrant: user 'kit' owns 'gitrepository-100046' yet getUtility(IGitRepositorySet).getByPath(kit, 'gitrepository-100046') is None (have tried with git_identity and other forms as well)
[02:05] <wgrant> blr: getByPath takes a user for privilege checking, and a full repository path.
[02:05] <wgrant> eg. the user should enter ~kit/project
[02:05] <wgrant> Or ~kit/project/+git/some-non-default-repo
[02:07] <blr> wgrant: is the interface comment misleading?
[02:07] <blr> s/comment/docstring/
[02:08] <wgrant> blr: Looks fine to me. It specifies the valid forms.
[02:08] <wgrant> What's confusing about it?
[02:09] <blr> '~kit/product-name-100049/+git/gitrepository-100046' returns a repo, however '~kit/+git/gitrepository-100046' does not
[02:09] <wgrant> blr: The latter is a personal repo; one not related to a project.
[02:09] <blr> aah
[02:09] <blr> of course, sorry.
[02:16] <blr> wgrant: pity zope IField doesn't support arbitrary attributes, or more specifically 'placeholder'
[02:16] <blr> default isn't quite the same
[02:17] <wgrant> blr: Right, that's more of an attribute of the widget, I think.
[02:17] <wgrant> eg. a widget could use the title as a placeholder.
[02:18] <blr> it would be nice to have the expected form for the git repo path as a placeholder, but perhaps I'll just add it the label
[02:18] <blr> I suppose I could set it in the js, but that might be a little odd.
[02:19] <blr> will be less of an issue once we have a search modal for git.
[02:20] <wgrant> Yep, search will make it much better. It's also a relatively uncommon case.
[02:33] <blr> wgrant: from [code]/project configure hosting is still for the series, do we want that to also point to project setbranch? That would leave project/[series] as the only means of configuring a series branch
[02:33] <wgrant> blr: I think that's very reasonable.
[02:34] <blr> yep, possibly even confusing in its current state.
[11:17] <cjwatson> wgrant: Nice, I didn't know about variable_name_prefix in batchnavs.
[11:18] <wgrant> cjwatson: I only knew about it from +members
[11:18] <wgrant> That's quite possibly the only other callsite.
[11:19] <cjwatson> wgrant: There's one other, BugTrackerSet:+index
[11:20] <wgrant> Ah, so there is.
[11:20] <cjwatson> https://bugs.launchpad.net/launchpad/+bug/1455907 suggests that the +members use doesn't work ideally, but that isn't really a batchnav problem as such.
[11:20] <mup> Bug #1455907: Every "Former members" batch page starts with a list of "Active members" <Launchpad itself:New> <https://launchpad.net/bugs/1455907>
[11:20] <wgrant> Yeah
[11:21] <cjwatson> Except in that maybe batchnav Next should do AJAX rather than reloading the whole page
[11:21] <wgrant> It can be fixed by anchors, only showing one half on subsequent batches, or using the AJAX batchnav stuff.
[11:23] <cjwatson> I see the FF for the last is disabled on prod
[11:23] <cjwatson> Indeed, everywhere
[11:24] <wgrant> I think AJAX batchnav is only enabled for +bugs
[11:24] <cjwatson> Branches too
[11:24] <wgrant> I'd forgotten it was even implemented for +branches until I saw the code.
[11:24] <cjwatson> But it's behind a disabled feature flag in both cases
[11:24] <wgrant> No, +bugs has been AJAXed since 2013.
[11:25] <cjwatson> Huh, that's interesting because I see ajax.batch_navigator.enabled mentioned in lib/lp/bugs/templates/bugs-listing-table.pt and that flag is definitely off.
[11:26] <wgrant> Is that the right template?
[11:26] <cjwatson> Perhaps not.
[11:26] <wgrant> Hm, I think it is.
[11:26] <wgrant> But there's a whole lot of custom JS for that page.
[11:40] <danilos> launchpad consistently times out when I attempt to comment on my MP (ajax just mentiones a server problem after ~30s, but if I go to +comment and try to submit, I get http://people.canonical.com/~danilo/tmp/lp-problem.png): any known issues?
[11:41] <danilos> MP in question is https://code.launchpad.net/~danilo/landscape-charm/actions/+merge/260916
[11:42] <wgrant> danilos: Yep, looks like a weird postgres issue, we're investigating.
[11:42] <danilos> wgrant, ok, thanks (I've even tried sending an email, still hasn't shown up)
[11:54] <cjwatson> danilos: The mail processor will be writing to the same table, indeed.
[11:54] <danilos> cjwatson, yeah, I was hoping the problem was somewhere between haproxy and lp appservers :)
[11:54] <cjwatson> No such luck.
[11:54] <danilos> yeah, got it
[11:58] <wgrant> danilos: Should all be OK again now.
[11:59] <danilos> wgrant, repeated comments have gotten through, fwiw (not a big deal, just letting you guys know)
[12:00] <wgrant> cjwatson: Oh, I didn't realise IGitCollection handled PersonProduct etc.
[12:01] <cjwatson> Yup, I overachieved slightly on the adapters since it was easy.
[12:55] <cjwatson> Top tip to self: it's more likely that somebody will review my merge proposals if I actually remember to propose them.
[21:01] <blr> wgrant: cjwatson: any suggestions for a user response for GitTargetError that is likely to make sense to them?
[21:01] <blr> "repo attached to another target" may not be meaningful
[21:21] <blr> hmm, well I'll go with that for now given it isn't the typical case.
[21:24] <cjwatson> blr: I think that your form field should be using a vocabulary that only offers repositories attached to the correct target.
[21:24] <blr> cjwatson: I think we decided to defer the repository search modal
[21:25] <blr> cjwatson: so the case here is just providing the user with a git_repository_location TextLine
[21:26] <blr> I believe the only error paths are repo not found, and target error
[21:28] <blr> cjwatson: just updating the configure code link on [code]/project, but perhaps you could have a look in review if there's a better way.
[21:29] <blr> I'll ping you
[22:20] <wgrant> blr: It could still have a non-searchable vocab.
[23:29] <blr> wgrant: ok, so presumably we want something like ValidTargetDefault?
[23:30] <wgrant> blr: Any Git repository for the target is valid.
[23:30] <wgrant> blr: What does the branch equivalent do? It has pretty much the same rules, but we don't need searching yet.
[23:32] <blr> hmm branch_location doesn't have a vocabulary afaict
[23:34] <wgrant> blr: How's it validated?
[23:34] <wgrant> It's possible that it isn't; there are some cross-project relations that shouldn't exist, but they might be from branches that have been moved later.
[23:36] <blr> wgrant: yep only validated for presence
[23:37] <wgrant> blr: OK, then it might not be worth using a vocab just yet. Manually validate the target and use an error message like "Repository is in a different project." or similar?
[23:37] <blr> okay, that's what I have atm
[23:38] <wgrant> Really weird that the branch case wouldn't use a vocab already!
[23:39] <blr> branch_type does