Perl 6 - the future is here, just unevenly distributed

IRC log for #opentreeoflife, 2015-08-13

| Channels | #opentreeoflife index | Today | | Search | Google Search | Plain-Text | summary

All times shown according to UTC.

Time Nick Message
11:51 jar286 joined #opentreeoflife
12:36 jar286 joined #opentreeoflife
13:35 kcranstn joined #opentreeoflife
14:39 jar286 joined #opentreeoflife
14:42 kcranstn joined #opentreeoflife
14:56 kcranstn joined #opentreeoflife
14:58 josephwb has emily been online here today?
15:03 josephwb hey jimallman
15:03 josephwb what do you think about notifying people when someone else has edited a study they curated?
15:04 josephwb notify + list changes
15:04 jar286 I think there may be an issue for this… it’s called a ‘watch list’
15:04 josephwb (finding changes is difficult)
15:04 josephwb ok, thanks
15:05 jar286 https://en.wikipedia.org/wiki/Wiki#Controlling_changes
15:06 josephwb i am going over the mapping changes emily did to "my" study
15:06 josephwb i left one taxon unmapped because it was not in the taxonomy, but it *is* in the new taxonomy
15:07 jar286 https://github.com/OpenTreeOfLife/opentree/issues/551
15:07 josephwb finding something like this after the fact will be near impossible without a complet remapping of a study
15:08 jar286 you might want to add a comment (upvote) to that issue
15:08 josephwb i am about to, thanks
15:08 jar286 i.e.: I agree with yu
15:08 jar286 s/yu/you/
15:15 josephwb https://github.com/OpenTreeOfLife/opentree/issues/551#issuecomment-130725958
15:20 jimallman sorry, coming late to the conversation. josephwb, the best we can do currently is the History tab, which will tell you who’s been editing and what comments they left. For a detailed review of changes, the diff view on GitHub is cryptic but definitive.
15:21 jimallman we could offer this in a link (diff from previous version, for each entry in History tab)
15:21 jar286 I think that’s a good idea
15:21 josephwb yeah
15:21 jimallman i see josephwb ’s comment though about poor/missing comments.
15:24 jimallman later, i suppose we could build a friendly “diff” viewer that loads two versions of the study Nexson, then compares them to list added/removed/changed stuff. i can add this to the issue, or make a new ‘enhancement’ issue for this…
15:29 jimallman for now, i’ll add the diff links to the History tab
15:34 josephwb thanks
16:51 jar286 joined #opentreeoflife
17:03 kcranstn joined #opentreeoflife
17:19 kcranstn joined #opentreeoflife
17:26 travis-ci joined #opentreeoflife
17:26 travis-ci OpenTreeOfLife/phylesystem-api#813 (link_to_study_diffs - 2bdc97e : Jim Allman): The build passed.
17:26 travis-ci Change view : https://github.com/OpenTreeOfLife/phylesystem-api/commit/2bdc97e542ff
17:26 travis-ci Build details : https://travis-ci.org/OpenTreeOfLife/phylesystem-api/builds/75477707
17:26 travis-ci left #opentreeoflife
17:55 pmidford2 joined #opentreeoflife
17:58 mtholder joined #opentreeoflife
17:59 mtholder jimallman, I did reply at https://github.com/OpenTreeOfLife/phylesystem-api/commit/2bdc97e542ff82d94fc32993bc03d478c925b7f5
17:59 mtholder I'm never sure who gets notified on those commit comments on GH.
17:59 jimallman mtholder: thanks! i’m following your responses.
17:59 jar286 joined #opentreeoflife
17:59 jimallman this is probably cleaner than my quick version. i’ll try to make that change.
18:00 mtholder I deleted the first 2 bogus comments.
18:00 * jimallman nods
18:00 jar286 https://github.com/pulls?user=OpenTreeOfLife
18:01 jar286 bottom to top (oldest to newest)
18:01 jar286 https://github.com/OpenTreeOfLife/opentree/pull/536 - stats ‘needs discussion’
18:02 jar286 conflicts
18:03 jar286 some discussion 13 days ago… do we need to discuss now, or move on?
18:03 kcranstn what still needs discussion?
18:04 travis-ci joined #opentreeoflife
18:04 travis-ci OpenTreeOfLife/phylesystem-api#816 (link_to_study_diffs - e215ce8 : Jim Allman): The build passed.
18:04 travis-ci Change view : https://github.com/OpenTreeOfLife/phylesystem-api/compare/2bdc97e542ff...e215ce86d451
18:04 travis-ci Build details : https://travis-ci.org/OpenTreeOfLife/phylesystem-api/builds/75482849
18:04 travis-ci left #opentreeoflife
18:04 jar286 I was just saying that the issue is tagged ‘needs discussion’.  I don’t know, was hoping jimallman could say
18:05 * jimallman is catching up now...
18:06 jimallman re: discussion, we’ve still had lots of spurious synthesis records in stats. pmidford2 was going to clean these up, i believe.
18:06 jimallman otherwise i think it’s ready for prime time.
18:07 jimallman (also, last i checked there are no stats on the production server)
18:07 pmidford2 Yes, I was.  I am dealing with some phylesystem issues first.
18:07 pmidford2 No, there are no stats on production
18:08 kcranstn any chance we can get this rolled out before publication?
18:08 jimallman should we wait on this until next week? or is cleanup possible today?
18:08 jimallman (what she said)
18:08 pmidford2 Yes, by publication, not likely today.  I'll aim for Monday to resolve this.
18:09 jar286 can’t the stats gathering code for phylesystem be pointed at old commits of the production phylesystem repo?
18:09 pmidford2 jar286 - not sure I understand
18:10 jar286 maybe I don’t understand.  do we have enough data to go live on production?
18:10 pmidford2 There is a history script that pulls the old commits out.  It needs to be run again.
18:11 kcranstn does that take five days?
18:11 kcranstn (I am also confused about what data is missing to go live)
18:11 pmidford2 No, though the script will take at least a day I think.
18:12 pmidford2 There is some historical data on the dev machine that we could go live with
18:13 kcranstn I would need more detail about “some historical data” to decide
18:14 travis-ci joined #opentreeoflife
18:14 travis-ci OpenTreeOfLife/phylesystem-api#817 (link_to_study_diffs - dc5f920 : Jim Allman): The build passed.
18:14 travis-ci Change view : https://github.com/OpenTreeOfLife/phylesystem-api/compare/e215ce86d451...dc5f920e2781
18:14 travis-ci Build details : https://travis-ci.org/OpenTreeOfLife/phylesystem-api/builds/75484178
18:14 travis-ci left #opentreeoflife
18:15 pmidford2 I ran the scripts back in January or Feburary, though I don't see the files on ot10.
18:15 kcranstn we have the necessary taxonomy data, we have synthesis data that includes many testing versions of synthesis, and we have phylesystem data that includes ???
18:15 kcranstn is that right?
18:16 pmidford2 We have phylesystem data for the past year or more, but only for dev.  Production hasn't been collecting data.
18:17 kcranstn aren’t we collecting phylesystem data from the github repo?
18:17 jar286 yes, that’s why I asked about getting the data from older commits.  which I guess you’re going to do.
18:17 kcranstn can you create an issue for what still needs to be done?
18:17 pmidford2 Yes, that's the script I need to run.  But that isn't the script that runs in the crontab
18:17 jar286 correct. that’s fine.
18:17 pmidford2 Yes, I will create an issue (or several)
18:18 kcranstn thanks
18:18 jar286 good
18:18 jar286 ok, shall we move on?
18:19 kcranstn yes, but I *really* want to close that PR next Thursday
18:19 kcranstn if not sooner
18:19 pmidford2 Ok, got it.
18:20 jimallman i’m around most of this weekend, if we want to test+deploy.
18:20 kcranstn https://github.com/OpenTreeOfLife/peyotl/pull/120
18:20 jimallman reminder: this is one of three related PRs
18:20 kcranstn are they all ready?
18:20 jimallman sadly, i need another day or two to get consistent behavior on all related pages
18:20 pmidford2 jimallman - sounds good
18:21 jar286 I’m going to be offline until next Wednesday, except for phone
18:21 jimallman but creation, modification, and deletion of collections are working well on the main Collections page
18:21 kcranstn have we seen the UI for those?
18:21 jimallman (not currently deployed on devtree)
18:22 jimallman kcranstn: no, UI was previous deployed on devtree, but has not been reviewed by others.
18:22 kcranstn maybe next week we can plan to deploy the collections branch on devtree in advance of this meeting
18:23 kcranstn or push the branch to development
18:23 jimallman yes, and i’ll call for comments ahead of time re: UI
18:23 kcranstn https://github.com/OpenTreeOfLife/germinator/pull/23
18:24 jar286 these tests have been running on each taxonomy build. i think this is not controversial. except for the bit that they were failing for treemachine, because the taxa in question are suppressed
18:24 kcranstn are these from the spreadsheets?
18:24 jar286 but for now I say we just deal with that, it’s a separate question
18:25 jar286 no
18:25 jimallman the comment on ‘Synedra acus’ is confusing (to me).. is this trying to propose a different id, or asking to confirm the existing id?
18:25 jar286 the comment should be deleted
18:25 jimallman (line 3. feel free to disregard)
18:25 kcranstn ok, merging?
18:26 jar286 ok with me
18:26 kcranstn https://github.com/OpenTreeOfLife/opentree/pull/687
18:27 kcranstn and related https://github.com/OpenTreeOfLife/deployed-systems/pull/15
18:27 jimallman these look good to me
18:27 jar286 I haven’t checked this but assume it’s fine
18:27 jimallman (i’ve seen the old and new behavior, just as described)
18:27 jar286 I wonder why we didn’t notice it before
18:27 kcranstn becuase once you have given github permission it never asks again
18:28 jimallman right, it’s not just logging out and back in.
18:28 kcranstn so you probably encountered this once many months ago and never again
18:28 jimallman right. i’ve clobbered my app registration on GitHub before for testing purposes, but not in a long time.
18:28 jar286 ah.
18:29 kcranstn so after you said “yes, opentree can have access” we bounced people back to github rahter than back to opentree
18:30 jar286 let’s just remember to re-test this after pushing to production today
18:30 kcranstn for https://github.com/OpenTreeOfLife/deployed-systems/pull/15 I didn’t change the historical config files in Cava, Bos, etc
18:31 jimallman agreed on testing. this used to work correctly, so i want to make sure it’s not a quirk of dev (or local server) vs. production
18:31 jar286 we should delete Bos et al.  I’ll make a to-do
18:31 kcranstn ok
18:33 jar286 looks good to me, shall I merge?
18:33 kcranstn somebody merging?
18:33 jar286 I’ll do it
18:33 jar286 both merged.
18:34 kcranstn thanks
18:34 jar286 https://github.com/OpenTreeOfLife/treemachine/pull/188
18:34 jar286 this looks ok to me
18:34 kcranstn me too
18:34 jimallman same
18:35 kcranstn merged
18:35 kcranstn josephwb - also deleting branch. that ok?
18:35 josephwb just a sec
18:35 josephwb yup
18:35 kcranstn https://github.com/OpenTreeOfLife/reference-taxonomy/pull/156
18:36 jar286 this isn’t necessarily the final 2.9, but I thought it ought to get moved to master to make it more visible
18:36 josephwb comments of the front page of a study in the curator are not registered as a change, so can't save
18:37 kcranstn I am ok merging 156
18:37 jimallman josephwb: noted, will try to reproduce (is there an issue for this?)
18:37 josephwb not yet
18:37 jar286 the inclusion tests are now part of the taxonomy build process
18:37 jimallman i’ll make one
18:38 josephwb making one now
18:38 jar286 i.e. they have been run… a couple are failing, but are not important and a very hard to fix
18:38 kcranstn merged. delete branch?
18:38 jar286 and I’m not sure they ever worked properly
18:39 jar286 feel free to merge 156
18:39 jar286 oh you did
18:39 jar286 reviewing https://github.com/OpenTreeOfLife/opentree/pull/692
18:39 kcranstn can I delete the branch?
18:39 kcranstn for 156?
18:40 jar286 yes
18:41 kcranstn nice, jimallman!
18:41 jimallman #692 (and related PR above) are a quick fix for josephwb’s request this morning. available for testing on devtree
18:41 jar286 692 is nice
18:41 kcranstn I think that fancier diffs are low priority
18:41 jar286 I took a look, it’s fine. I say merge
18:41 kcranstn github diffs are likely better than what we could write
18:41 jimallman :)   note that some diffs won’t appear in the Github UI, since files are (relatively) huge and changes many
18:42 kcranstn but those tend to be made by people who have mad git skillz
18:42 jar286 I’ll merge
18:42 jimallman it’s enough context to enable gathering the raw data and diff’ing elsewhere, if needed
18:42 kcranstn exactly
18:42 jar286 someone beat me to it
18:43 kcranstn https://github.com/OpenTreeOfLife/phylesystem-api/pull/155
18:43 jimallman support for building diff URLs, from phylesystem-api
18:44 kcranstn looks good
18:44 jimallman this just adds the study’s shard name to the “full JSON” version sent to the study editor
18:45 jar286 I don’t really get how this works.  how does _fetch_shard_name get called?
18:45 jimallman it’s called in the study API’s GET method (line 495)
18:46 jimallman https://github.com/OpenTreeOfLife/phylesystem-api/pull/155/files#diff-0803c6285c2b18bd9e1e677d1df1bbceR496
18:46 jar286 i see… and _fetch_duplicate_study_ids is an ajax call?
18:46 jar286 (or service?)
18:47 jimallman … along with check for other supplemental info. this is all local on the API server, which has its local phylesystem (and local peyotl) to provide this information
18:47 jar286 hmm… so this is buried inside some other service
18:47 jar286 ok, no matter.
18:48 jimallman there’s a module ‘api_utils’ that provides this: https://github.com/OpenTreeOfLife/phylesystem-api/blob/master/modules/api_utils.py#L45
18:48 jar286 and this is used by the diff feature, right? …
18:48 jar286 so (a) it’s tested (b) it’s essential.
18:48 jimallman yes, to retrieve the shard name (which we need to build the diff URL)
18:49 jar286 so we should merge it
18:49 jimallman both tested and essential, so we’re ready for a multi-shard phylesystem when the day comes.
18:49 kcranstn let the studies come!
18:49 jar286 merged
18:49 kcranstn thanks
18:50 jar286 the end
18:50 kcranstn I have a question before we go
18:50 jar286 at 3:30 we’ll start deploying
18:50 jar286 yes?
18:50 kcranstn what is the status of the code providing these pages:
18:50 kcranstn https://devtree.opentreeoflife.org/about/taxonomy-version/ott2.9
18:50 kcranstn and
18:50 kcranstn https://devtree.opentreeoflife.org/about/synthesis-release/2015-08-13
18:50 kcranstn and associated About menu items?
18:51 kcranstn is that buried in with that statistics PR?
18:51 kcranstn If the taxonomy page is ready, can we push that out?
18:51 travis-ci joined #opentreeoflife
18:51 travis-ci OpenTreeOfLife/phylesystem-api#819 (master - 8098bf7 : Jonathan A Rees): The build passed.
18:51 travis-ci Change view : https://github.com/OpenTreeOfLife/phylesystem-api/compare/54baf1265539...8098bf79559a
18:51 travis-ci Build details : https://travis-ci.org/OpenTreeOfLife/phylesystem-api/builds/75488949
18:51 travis-ci left #opentreeoflife
18:52 kcranstn (although there is still a TODO at the bottom of the page)
18:52 jimallman hm, i think these pages are in the stats PR
18:52 kcranstn I like that taxonomy release page
18:52 kcranstn and I like having a link in the About menu
18:52 jar286 funny, the taxonomy stats are for 2.8, I think
18:53 jar286 funny in that it’s juxtaposed with 2.9 release notes
18:53 jimallman if you step to previous versions, do the stats make sense? maybe i have some kind of off-by-one error here.
18:53 jimallman kcranstn: thanks, will sweep for TODOs in this branch
18:54 kcranstn can we pull out the taxonomy page and menu item into a separate PR?
18:55 jimallman note that there are no release notes for ott2.7. can we put something simple together to avoid this message?
18:55 jimallman https://devtree.opentreeoflife.org/about/taxonomy-version/ott2.7
18:55 jar286 I think we’re talking about two different drafts of 2.9.  The discrepancy is evident by comparing the first date with the ‘is not released yet’ line in the release notes
18:56 jar286 seems safer to not mention 2.9 at all until it’s released, maybe
18:56 jimallman kcranstn: re: making a separate PR, it’s thorny but possible.
18:56 jar286 esp. since there’s no deployed TNRS for 2.9 yet
18:56 kcranstn if the stats PR is getting resolve next week, then I am ok leaving it as is
18:56 kcranstn resolved
18:57 kcranstn but in the future, let’s try and keep the PRs smaller
18:57 kcranstn that one is a behemoth
18:57 jimallman agreed on small PRs, this one kind of sprawled
18:57 pmidford2 joined #opentreeoflife
18:57 jimallman so should a taxonomy release become “real” (visible in stats) only after it’s been used in synthesis? if so, we should have the data to support that test.
18:58 kcranstn no, we can release taxonomy separately from synthesis
18:58 kcranstn but the release should include availability via API (incl TNRS)
18:58 jar286 right, 2.9 will be released long before there’s a new synthesis
18:58 jar286 I’ll make simple release pages for 2.7 and 2.5 based on git commit logs…
18:59 kcranstn thanks, jar286
18:59 jimallman is this (release for API/TNRS) something i can detect programmatically, or should we mark this explicitly in the stats? (or should we not gather stats for taxonomy until this release happens?)
19:00 jar286 I don’t know
19:00 jar286 there might be an API call that will tell you
19:00 jimallman ok, will check for this
19:00 jar286 doesn’t look like it.
19:01 jar286 https://github.com/OpenTreeOfLife/opentree/wiki/Open-Tree-of-Life-APIs#tnrs
19:01 jimallman will API and TNRS always update to a new taxo version in tandem?
19:01 kcranstn there should be a call for that
19:01 kcranstn can you make an issue?
19:01 jar286 oh yes there is
19:01 jar286 the taxonomy version is returned in every match_names call
19:01 kcranstn cool
19:02 jar286 https://github.com/OpenTreeOfLife/opentree/wiki/Open-Tree-of-Life-APIs#match_names
19:03 jimallman so… in principle, i could do a TNRS check, get its taxo version, and show only that AND EARLIER versions in the stats and About pages
19:03 jar286 in principle. yes.
19:03 jimallman roundabout, but workable
19:03 jar286 seems like a lot of scripting for events that happen only a few times a year
19:03 kcranstn I was thinking the same
19:04 kcranstn feels like taxonomy and synthesis release pages can tolerate some manual work
19:04 jimallman agreed. if possible, i’d prefer to mark the taxo stats directly, or skip gathering these stats until a proper release
19:06 jimallman i forgot. the taxonomy stats are already manually created:  https://devtree.opentreeoflife.org/static/statistics/ott.json
19:06 jar286 this doesn’t make sense to me: “Sources: TODO: Link to synthesis releases using this taxonomy?”
19:06 jar286 this suggests synthesis releases are sources, but they’re not
19:07 jimallman each synthesis release uses a specific version of ott, yes?
19:07 jimallman i thought we might try to tie the two together here
19:07 jar286 the taxonomy is a source for synthesis, not the other way around
19:07 jar286 I think it’s cleaner to not mention synthesis on the taxonomy pages. want to emphasize their independence
19:08 jimallman ok, will remove this TODO, and its counterpart on the Synth Release page:  https://devtree.opentreeoflife.org/about/synthesis-release/2015-08-13
19:08 jar286 wait, taxonomy *should* be mentioned for synthesis pages
19:08 jar286 it’s asymmetric
19:09 jimallman i understand that taxonomy is an input to synthesis, and not vice versa
19:09 jimallman i thought it was useful / interesting to point out the relationship from both sides
19:09 jar286 so “TODO: Link to taxonomy release used in this synthesis?”  - yes
19:10 jar286 if there’s a link from the taxonomy side, it has nothing to do with ‘sources’
19:10 jar286 it could say, ‘synthetic trees that have this taxonomy version as input’
19:10 jimallman i see, the problem is with the Sources heading. gotcha.
19:11 jimallman i stopped reading that a long time ago, probably cloned from the synth-release page  :-/
19:11 jimallman alternate suggestions for this heading welcome
19:14 jar286 jimallman, which directories should be backed up for the stats feature?
19:15 jar286 pmidford2, I think you need the ssh key for production, yes?
19:15 jimallman devtree:repo/opentree/webapp/static/statistics/
19:15 pmidford2 Yes, I think that was the issue.
19:16 jar286 do you have a pgp key?
19:16 pmidford2 No, I still don't
19:18 jar286 everyone should have a pgp key
19:19 kcranstn joined #opentreeoflife
19:19 jar286 I’ve put it on ot10, in ~opentree.  Copy it from there to your own machine, then delete it from ot10
19:21 kcranstn I’ll be away for a bit. Have engineer at house looking at floor joists
19:22 * jimallman is breaking for late lunch, unless i should stay
19:22 jar286 sounds bad
19:23 jar286 pmidford2, did you get what I said above?
19:24 pmidford2 yes, but I don't see it on ot10
19:24 pmidford2 Never mind, got it
19:25 pmidford2 jar286 - ok, grabbed and deleted.  Also had another look at installing pgp.
19:27 jar286 ok good.  just be careful with that server…  fine to populate the stats directory and set up cron, then after the PR is merged (later) it should be live
19:27 jar286 right?
19:32 pmidford2 Got it.
19:33 pmidford2 I'll be out for a bit, but I've posted two issues for statistics for people to review.
19:34 jar286 ok
19:47 kcranstn joined #opentreeoflife
19:53 kcranstn joined #opentreeoflife
20:01 kcranstn joined #opentreeoflife
20:24 jar286 updating production now
20:24 kcranstn excellent
20:25 jar286 all done. time for any tests that should be done
20:25 kcranstn I will test the authentication
20:25 jar286 hmm, no diff links
20:26 jimallman was the api PR merged?
20:26 jimallman phylesystem-api, i mean?
20:26 jimallman i’ll take a look
20:27 kcranstn something is funky with authentication
20:27 kcranstn sending repeated requests with different states
20:28 jimallman re: diff links, the response to GET study does not have the new ‘shardName’ property: https://api.opentreeoflife.org/phylesystem/v1/study/ot_83?output_nexml2json=1.0.0&auth_token=ANONYMOUS
20:28 kcranstn did the config file on production get changed?
20:28 kcranstn for the github redirect?
20:29 kcranstn getting a “redirect_uri_mismatch” error
20:29 jimallman ok, diff links are good.
20:29 jimallman kcranstn: i’ll check the config file..
20:29 jar286 Yes, I updated the deployed-systems repo before pushing
20:30 jar286 and I thought we merged the PR
20:30 jimallman joined #opentreeoflife
20:31 jar286 oh.  maybe github needs to be told about the change
20:31 jimallman ah, for the production apps, yes
20:31 kcranstn right...
20:31 kcranstn on that
20:32 kcranstn done
20:32 jimallman i just took a peek on production (tree.opentreeoflife.org). webapp/private/config looks right (no ”/login” in the redirect_uri)
20:32 kcranstn working now
20:33 kcranstn I forgot that I only changed dev initially
20:33 kcranstn webapp and curator tested and behaving as expected
20:33 jimallman it’s odd for me. if i click Login, it looks like i’m logging in, but i return to the same page with the Login link still showing.
20:34 jimallman (this is from the curation app, will try again from synth-tree viewer)
20:34 kcranstn and you revoked access from your github account?
20:34 kcranstn ah, I see what you mean
20:34 jimallman have not revoked yet.
20:34 kcranstn yes, me too. Still says “Login” in corner, not my username
20:35 jimallman same result in synth-tree viewer and curation app. odd.
20:35 jimallman i’ll take a closer look at oauth results
20:35 kcranstn ok
20:38 kcranstn I am getting a CORS error
20:39 kcranstn not sure if that is related
20:41 jimallman hm, tips to reproduce?
20:41 kcranstn have Console open in Firefox devtools, hit Login
20:41 kcranstn in curator
20:42 kcranstn same in webapp
20:42 kcranstn Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://assets-cdn.github.com/assets/github/index-55d89b4ebef0905d740bb0006ae466934d43cfa6f709e983b6a4b584236d848c.css. (Reason: CORS request failed).
20:44 kcranstn I am getting the same behaviour on dev
20:46 jimallman i’m not seeing the CORS errors here (in Chrome or Firefox). i have noticed regular requests to assets-cdn.github.com though… not sure what’s up there
20:48 kcranstn argh
20:48 kcranstn we may need to back out of this change
20:50 kcranstn it feels fragile that the redirect_url is set in so many places
20:50 jimallman if you’re getting the same behavior on dev and production, that should mean we can reproduce it on dev and track it down
20:50 kcranstn what happens when you log in on dev/
20:51 kcranstn ?
20:51 jimallman just a sec
20:52 jimallman same result (Chrome): click Login, no apparent errors, but i still need to Login…
20:53 kcranstn ok, so how to debug this?
20:53 jimallman i noticed the code and state values coming back on a query-string (redirect_uri)...
20:54 jimallman i tweaked the url to match (i believe) the old settings, and it logged me in properly:  https://devtree.opentreeoflife.org/user/login?code=5c018508ec038f62effe&state=18e8ac344e369990bc0d62c05c8f7f39
20:55 kcranstn the login url?
20:55 kcranstn sorry, confused
20:55 jimallman trying again… will post more details
20:55 jimallman so i’ve logged out (in devtree curation homepage)
20:57 jimallman final URL in browser (after apparently-successful auth) is https://devtree.opentreeoflife.org/curator?code=66780d96cb3f9e349030&state=aa257d58e07ac2dbc78551d6d67ac487
20:57 jimallman this is the new redirect_uri, i believe.
20:57 jimallman i think the problem is that this page doesn’t know what to do with code and state vars, which is why we formerly redirected to the login page
20:58 kcranstn can we change this page to deal wiht the code and state vars?
20:58 kcranstn rather than bounce people back to github?
20:58 jimallman if i tweak the URL above to be ‘https://devtree.opentreeoflife.org/curator/user/login?code=…' it works
20:59 kcranstn manually tweak it
20:59 jimallman possibly, but the web2py login page is special and has been patched to handle these oauth values
20:59 jimallman yes, i manually tweaked the URL above to verify
21:00 kcranstn yes, that works for me too
21:00 jimallman odd, because that’s not quite the old redirect_uri either, is it?
21:01 jimallman i thought it just had an extra “/login”, not “/user/login”...
21:01 kcranstn nope, user/login
21:01 jimallman ah, good
21:01 jimallman (at least that makes sense)
21:02 jimallman we can either try to teach our preferred landing page to handle code and state (as you suggested above) or teach the login page not to bounce to GitHub for no reason. i should have tried to examine this more closely before.
21:04 kcranstn but I think in the short term, we (unfortunately) need to revert this
21:04 jimallman agreed, it’s going to take a little while to sort this out. will it present a problem for users who are already logged in?
21:04 kcranstn which involves an unfortunate number of places where we have that URL set
21:05 kcranstn jar286, you hearing this?
21:05 jar286 catching up
21:07 jar286 hmmph.
21:07 kcranstn I agree
21:08 * jimallman is meanwhile reviewing the OAuth code…
21:09 jar286 we can just go backward two commits, since the study diffs hyperlink appears not to work
21:10 kcranstn ok
21:10 jimallman diff links work for me
21:10 jar286 ah, maybe a caching problem… let me see
21:10 jimallman (started working, after my initial failed attempt)
21:11 kcranstn diff links work for me too
21:13 jar286 me too.
21:14 * jar286 starting to look at man git-reset
21:15 jar286 could do git reset + git cherry-pick
21:15 jimallman jar286: or perhaps ‘git revert $SHA’?
21:16 jar286 yes, that’s bettere
21:16 jar286 would you like to do it?… I’d like to modify the master branch on github, not just patch the running instance
21:17 jimallman agreed. just a sec. (pretty sure we can do it on master, esp. as it’s just one commit)
21:18 jar286 yes, should be very simple
21:19 jimallman done: https://github.com/OpenTreeOfLife/opentree/commit/96d25f2d9e87bd0d71b05d04589a5bbedd4d11a8
21:20 jimallman …and of course we’ll need to repeat the tweaks to config files and GitHub’s app information… anywhere else?
21:20 jar286 I can revert deployed-systems and push to production
21:20 kcranstn don’t the config files get written?
21:20 kcranstn I’ll do the github apps
21:20 kcranstn (already have the page loaded)
21:21 kcranstn updated
21:21 jimallman yes, config files should be fresh, and should follow the server-config files (the code we just reverted us only used to set default redirect_uri values if there’s nothing in the server-config file)
21:22 jimallman we have so many places to touch, partly because of this belt-and-suspenders approach. i suppose we could simplify by taking away the defaults and throwing an error instead.
21:23 jar286 I’ve been summoned home so can’t do the deployed-systems change right now, sorry
21:24 jar286 can y’all take care of this? I think I can be back around 7
21:24 jimallman yes, i’ve got it.
21:24 kcranstn ok
21:24 * jimallman is grabbing coffee, brb
21:27 kcranstn joined #opentreeoflife
21:30 jimallman ok, changes reverted in deployed-systems/master
21:31 jimallman now i should be able to deploy webapps to production…
21:31 jimallman (i don’t think the larger deployment checklist comes into play for this)
21:32 kcranstn ok
21:34 kcranstn I’ve never deployed production, but happy to be walked through it
21:34 jimallman done. i’m seeing normal login behavior (haven’t cleared my GitHub app info yet) in the curation app...
21:34 kcranstn same here
21:35 jimallman but synth-tree viewer “logs in” to GitHub home page (your original report)
21:35 jimallman odd
21:35 kcranstn yup
21:35 kcranstn we can trouble shoot more tomorrow
21:35 kcranstn but cleaning this up would be good
21:36 jimallman agreed. chasing the oauth request-chain now...
21:36 jimallman yeehaw!
21:36 kcranstn finding the tree.opentreeoflife.org URL is not that straightforward, so I hate to leave people stranded at github
21:38 jimallman agreed. original login request has a sensible redirect_uri, but it gets mangled in the response to ‘https://tree.opentreeoflife.org/user/login?code=47e…'
21:39 jimallman this is throwing a ‘303 SEE OTHER’ response with the Location header set to ‘https://github.com/'. odd.
21:41 jimallman it seems i successfully logged in, but was bounced needlessly to the GitHub homepage
21:42 kcranstn yup - that’s the behaviour that started me down this path
21:46 jimallman comparing the chain of login requests from curation shows a better outcome. the same 303 redirects (two in a row), but the second one preserves the original destination page (not the redirect_uri, but the study page or whatever).
21:46 jimallman i’ll retrace our steps, maybe this is missing or messed up in the synth-tree viewer.
21:48 jimallman of course! this should have been part of the original login request. not sure why it’s not (or no longer) there...
21:49 jimallman https://tree.opentreeoflife.org/user/login  should look more like  https://tree.opentreeoflife.org/user/login?_next=http://
21:50 kcranstn not sure I follow
21:51 jimallman we didn’t specify a destination page (“go here after i log in”), so it apparently defaults to GitHub (oddly) rather than the current page.
21:51 kcranstn we in web2py, you mean?
21:52 jimallman or in the client-side code, whoever sets the initial login URL in the blue ’Login’ hyperlink in the comment tool
21:54 jimallman here’s one place we modify this URL, after moving through the synth-tree:  https://github.com/OpenTreeOfLife/opentree/blob/51a96f3885c5eecc3a14547d6c664a0e714d49b8/webapp/static/js/treeview.js#L596-L614
21:55 jimallman i’ll check to see that this is still running, maybe our recent changes have confused it
21:56 jimallman here’s where we create the link (in plugin_localcomments), but i this doesn’t specify the ‘?_next=blah’ part: https://github.com/OpenTreeOfLife/opentree/blob/51a96f3885c5eecc3a14547d6c664a0e714d49b8/webapp/controllers/plugin_localcomments.py#L505
21:58 jimallman testing now in local version...
22:01 jimallman ok, i’ve found the problem. when the JS function above (fixLoginLinks), the Login link hasn’t been created yet. it’s not around until the comment form appears, so i’ll need to call it again each time we create or modify this form.
22:02 jimallman this may have been broken for quite some time. maybe another reason we don’t get much feedback from this UI.
22:02 kcranstn that seems inelegant
22:02 jimallman agreed, but we’re moving through the tree, and the login needs to “follow along” from node to node.
22:03 jimallman the page tracks all this state as it manages the browser history, so the information is close at hand.
22:04 jimallman we do the same trick in the curation app, to track changing tabs, etc and always return to the same place
22:04 jimallman after login, that is
22:04 kcranstn ok, got it
22:06 jimallman here’s the change i was going to make (already here), so something has gone wrong: https://github.com/OpenTreeOfLife/opentree/blob/master/webapp/static/js/treeview.js#L223
22:07 kcranstn boo
22:20 jimallman for the record, here’s a fix: https://github.com/OpenTreeOfLife/opentree/pull/697
22:55 jimallman also for the record, entered these findings in the original GitHub issue:  https://github.com/OpenTreeOfLife/opentree/issues/685#issuecomment-130871740

| Channels | #opentreeoflife index | Today | | Search | Google Search | Plain-Text | summary