#duraspace IRC Log


IRC Log for 2017-04-19

Timestamps are in GMT/BST.

[1:16] * Guest94491 (~pionenmat@dansemusique.net) has joined #duraspace
[1:31] * Guest94491 (~pionenmat@dansemusique.net) Quit (Quit: WeeChat 0.3.7)
[6:37] -card.freenode.net- *** Looking up your hostname...
[6:37] -card.freenode.net- *** Checking Ident
[6:37] -card.freenode.net- *** Found your hostname
[6:37] -card.freenode.net- *** No Ident response
[6:37] * DuraLogBot (~PircBot@webster.duraspace.org) has joined #duraspace
[6:37] * Topic is 'Welcome to DuraSpace IRC. This channel is used for formal meetings and is logged - http://irclogs.duraspace.org/'
[6:37] * Set by tdonohue on Thu Sep 15 17:49:38 UTC 2016
[12:13] * mhwood (mwood@mhw.ulib.iupui.edu) has joined #duraspace
[12:59] * tdonohue (~tdonohue@dspace/tdonohue) has joined #duraspace
[13:55] * misilot (~misilot@p-body.lib.fit.edu) Quit (Remote host closed the connection)
[14:53] * misilot (~misilot@p-body.lib.fit.edu) has joined #duraspace
[15:02] <tdonohue> For anyone looking for the DSpace Developers Meeting, unfortunately our Slack <-> IRC integration seems to have crashed overnight (just noticed it) and I cannot get it restarted.
[15:05] * ntorres (c1895861@gateway/web/freenode/ip. has joined #duraspace
[15:05] * jcreel (~jcreel@jcreel.tamu.edu) has joined #duraspace
[15:05] * terry-b (~chrome@75-172-43-243.tukw.qwest.net) has joined #duraspace
[15:06] <tdonohue> Ok, so, I've just pinged everyone on Slack to move over here. Giving folks a few minutes.
[15:06] <tdonohue> In the meantime, here's the agenda for today: https://wiki.duraspace.org/display/DSPACE/DevMtg+2017-04-19
[15:06] <kompewter> [ DevMtg 2017-04-19 - DSpace - DuraSpace Wiki ] - https://wiki.duraspace.org/display/DSPACE/DevMtg+2017-04-19
[15:07] * PhilipV (~philip@ has joined #duraspace
[15:07] <tdonohue> Not sure what happened to the Slack sync bot, but it currently seems to be blocked from entering this IRC channel. Hence, the sync isn't working. So, old school IRC it is!
[15:08] <tdonohue> Ok, as usual, a reminder we have our next DSpace 7 Meeting tomorrow at 15UTC (11am EDT). It's in Google Hangouts this week
[15:09] * hpottinger (~hpottinge@ has joined #duraspace
[15:09] <tdonohue> We're still working on getting 6.1 out the door (essentially ASAP, or whenever we close out our remaining high priority tickets)
[15:09] <tdonohue> Those remaining 6.1 tickets are listed at: https://jira.duraspace.org/issues/?jql=filter%20%3D%2013904%20AND%20fixVersion%20%3D%206.1%20ORDER%20BY%20fixVersion%20DESC%2C%20priority%20DESC%20%20%20
[15:09] <kompewter> [ Issue Navigator - DuraSpace JIRA ] - https://jira.duraspace.org/issues/?jql=filter%20%3D%2013904%20AND%20fixVersion%20%3D%206.1%20ORDER%20BY%20fixVersion%20DESC%2C%20priority%20DESC%20%20%20
[15:11] <tdonohue> *most* of these have code written / in review stages. There's a few tickets that are still flagged as "Needs Volunteer". If those two don't end up with volunteers soon, we may simply reschedule them for a 6.2. However, everything in Code Review Needed status needs closing out for 6.1
[15:11] <tdonohue> As the list is small, we'll do a quick run through of status...and see if we can pick up a new volunteer along the way
[15:12] <tdonohue> First up, we have DS-3558 / DSPR#1707
[15:12] <kompewter> [ https://github.com/DSpace/DSpace/pull/1707 ] - DS 3558 Case-insensitive bot matching option by Frederic-Atmire
[15:12] <kompewter> [ https://jira.duraspace.org/browse/DS-3558 ] - [DS-3558] Case-insensitive bot user agent matching can have performance impact - DuraSpace JIRA
[15:13] <tdonohue> The code changes here seem relatively minor to me (and provide major performance improvements)... I had a comment that was fixed.
[15:13] <tdonohue> It really just needs another review, and possibly a tester to give it a sanity check. Anyone interested in helping with either?
[15:14] <hpottinger> I'll take that on
[15:15] <tdonohue> Thanks hpottinger. I've assigned the PR to you
[15:15] <tdonohue> next up in our list is DS-3552 / DSPR#1694
[15:15] <kompewter> [ https://github.com/DSpace/DSpace/pull/1694 ] - Ds 3552 read only context and hibernate improvements by tomdesair ¡ Pull Request #1694 ¡ DSpace/DSpace ¡ GitHub
[15:15] <kompewter> [ https://jira.duraspace.org/browse/DS-3552 ] - [DS-3552] Select Collection step and submissions page load very slow on large repositories due to Hibernate - DuraSpace JIRA
[15:16] <tdonohue> This has turned into a much larger change (refactor of Context object to include support for *read-only* context objects). But, it seems very important, and also fixes several performance issues (including DS-3559)
[15:16] <kompewter> [ https://jira.duraspace.org/browse/DS-3559 ] - [DS-3559] OAI Indexing extremely slow and memory inefficient for bigger amount of items - DuraSpace JIRA
[15:17] <tdonohue> As it is larger, it could use some more eyes on it. I admit, I reviewed an earlier version of this, but it's been enhanced & improved much since then
[15:17] <mhwood> Just got in from a conflicting meeting....
[15:18] <hpottinger> I think mhwood just volunteered ;-)
[15:18] <tdonohue> This *was* tested by Christian Scheible to ensure it resolves the Ds-3559 ticket, and it does. But more eyes/reviews here are probably warranted, because of the extent of the changes
[15:20] <mhwood> I will read it.
[15:20] <tdonohue> Oh, and this fixes some of our Hibernate Ehcache configs too. ;)
[15:20] <terry-b> I am curious to check this one out, but I will not be able to get to it for a couple weeks. I am intrigued to see how it is managing what is initialized from hibernate.
[15:20] <tdonohue> Thanks mhwood
[15:21] <tdonohue> and terry-b, if you find time, great!
[15:21] <hpottinger> I've got it on my list, but I can't guarantee I'll get to it any time soon
[15:21] <tdonohue> I will also give this one a review of the code. This is already somewhat "tested" by unit tests, but we do need to ensure we understand this code and approve of the direction here
[15:22] <tdonohue> And as noted by Tom Desair, there likely are other areas of the code that should/could make use of this new READ-ONLY Context option...as it provides much better performance. The new REST API could also likely use this
[15:23] <tdonohue> (for read-only REST options that is)
[15:23] <hpottinger> capture that in a ticket?
[15:24] <tdonohue> hpottinger: it's already mentioned in DS-3552 comments (see the last few comments by Tom). We can split it out into separate tickets once we approve this code
[15:24] <kompewter> [ https://jira.duraspace.org/browse/DS-3552 ] - [DS-3552] Select Collection step and submissions page load very slow on large repositories due to Hibernate - DuraSpace JIRA
[15:25] <tdonohue> Tom notes several areas of the code that could likely benefit here...but we'd need to do each separately
[15:25] <tdonohue> So, while this is an important ticket, I'm going to move along now
[15:26] <tdonohue> Next up here is DS-3572 / DSPR#1715
[15:26] <kompewter> [ https://jira.duraspace.org/browse/DS-3572 ] - [DS-3572] AuthorizeService.authorize(..., EPerson, ...) checks context.currentEPerson instead of specified one - DuraSpace JIRA
[15:26] <kompewter> [ https://github.com/DSpace/DSpace/pull/1715 ] - DS-3572: Check authorization for a specified user instead of currentUser by pnbecker
[15:27] <tdonohue> This is an important API bug / confusingly named method found by pbecker. He noticed an accidental misuse of this method in another area of the API because of the confusion around the method's purpose
[15:27] <tdonohue> So, this is both an API refactor & a bug fix
[15:28] <tdonohue> Huh...and the PR seems to be stalled in it's Travis build. Weird, Travis never even started
[15:28] <terry-b> I have a TODO to test a handful of PR's from pnbecker
[15:29] <tdonohue> Yes, while we are on the subject here, pnbecker has a second (smaller) PR that also should get into the release... it's a "quick win": DSPR#1714
[15:29] <kompewter> [ https://github.com/DSpace/DSpace/pull/1714 ] - DS-3575: Rename misguiding find method in ResourcePolicyService by pnbecker ¡ Pull Request #1714 ¡ DSpace/DSpace ¡ GitHub
[15:30] <tdonohue> I've already approved 1714, but was waiting for another set of eyes
[15:32] <mhwood> +1 beginning to document one of the new interfaces.
[15:33] <tdonohue> Ok, so both of pbecker's PRs could use eyes. I plan to review 1715 when I get the chance...and I can merge 1714 if someone else gives it a look and approves
[15:34] <mhwood> I will go through 1714 again and say something.
[15:34] <tdonohue> moving along here.. we have two 6.1 tickets that still are in "needs volunteer" status. DS-3447 and DS-3287. I'm not going to say anything about those other than the reminder that they will be rescheduled if we don't find a volunteer soon.
[15:34] <tdonohue> Thanks mhwood
[15:34] <kompewter> [ https://jira.duraspace.org/browse/DS-3447 ] - [DS-3447] Transition ORCID integration to ORCID API 2.0 - DuraSpace JIRA
[15:34] <kompewter> [ https://jira.duraspace.org/browse/DS-3287 ] - [DS-3287] ElasticSearch Statistics fails in 6.0 (does not work at all) - DuraSpace JIRA
[15:35] <tdonohue> next up is DS-3406 / DSPR#1684
[15:35] <kompewter> [ https://jira.duraspace.org/browse/DS-3406 ] - [DS-3406] Sub-communities and collections not sorted alphabetically - DuraSpace JIRA
[15:35] <kompewter> [ https://github.com/DSpace/DSpace/pull/1684 ] - DS-3406: Sort communities and collections in-memory using a comparator by tomdesair
[15:35] <hpottinger> 1714 is now at +2
[15:35] <tdonohue> This one has some comments/testing in the ticket (Ds-3406). It sounds like there may still be some minor sorting problems here?
[15:35] <tdonohue> thanks hpottinger
[15:36] <tdonohue> Although, now I see the comments on Ds-3406 say the sorting is fine except that strangely it is slightly different in Mirage2
[15:37] <tdonohue> I'm not sure myself if Mirage2 is simply doing something different here (in Javascript or similar) than other themes...so, maybe that is expected?
[15:39] <tdonohue> I added a comment to that ticket asking Tom Desair if he knows whether Mirage2 behavior is correct
[15:40] <tdonohue> Overall though, to me 1684 looks pretty straightforward...and it has Unit tests. So, it *seems* like it's nearly ready. We just need to verify the behavior here, and get some reviewers to add +1s (if they approve)
[15:42] <tdonohue> Last in the 6.1 list is DS-3564
[15:42] <kompewter> [ https://jira.duraspace.org/browse/DS-3564 ] - [DS-3564] Change default value for db.maxidle - DuraSpace JIRA
[15:42] <tdonohue> And it looks like this is a tiny change...and mhwood has claimed it
[15:42] <mhwood> Yes, I have a note that I was supposed to check it out.
[15:42] <tdonohue> 3564 seems like an obvious quick win though...and I'd support it
[15:43] <terry-b> I agree
[15:44] <mhwood> I'll get a PR in this week. It should, indeed, be small and simple.
[15:44] <tdonohue> Thanks mhwood
[15:44] <hpottinger> mhwood++
[15:45] <tdonohue> So, are there other tickets/PRs that folks want to highlight here today? I know we also have a decent list of "quick win" PRs that are waiting for merger...and we might be able to dig into some of those in the Backlog Review Hour (next hour)
[15:45] <hpottinger> quickwins++
[15:47] <tdonohue> Ok, not hearing any other tickets to highlight. In that case, here's our listing of "quick win" PRs: https://github.com/DSpace/DSpace/pulls?q=is%3Apr+is%3Aopen+label%3A%22quick+win%22
[15:47] <kompewter> [ Pull Requests · DSpace/DSpace · GitHub ] - https://github.com/DSpace/DSpace/pulls?q=is%3Apr+is%3Aopen+label%3A%22quick+win%22
[15:47] <tdonohue> (I'll admit, it is also possible that I've "overlooked" a "quick win" in this list... I *try* to quickly review PRs as they come in and flag as "quick win" if they look smaller / easier to test. But, if I overlook any, please ping me or flag as "quick win" yourself)
[15:48] <tdonohue> The first few "quick win" PRs on this list were mentioned earlier...both are from pbecker and fix some obvious API bugs
[15:49] <tdonohue> As we already got some votes/volunteers on those, I'll skip them. But more eyes are welcome on them, as I think these are both important
[15:49] <terry-b> I plan to test https://github.com/DSpace/DSpace/pull/1709 as well. Does the merge to 5x trigger us to do the cherry-pick to 6x and master?
[15:49] <kompewter> [ DS-3516 ImageMagick PDF Thumbnail class should only process PDFs by alanorth · Pull Request #1709 · DSpace/DSpace · GitHub ] - https://github.com/DSpace/DSpace/pull/1709
[15:51] <tdonohue> terry-b: yes...any PR that is applicable to multiple branches should be cherry-picked elsewhere. I *suspect* 1709 should cherry-pick cleanly to 6.x and master though, as those classes were not refactored much (if at all) in 6.x API
[15:51] <tdonohue> But, I'm glad to merge & cherry-pick it myself...just ping me once tested
[15:51] <terry-b> I would like to make a pitch for the IIIF discussion that we will hold on Friday: See https://wiki.duraspace.org/display/DSPACE/IIIF+and+DSpace for details.
[15:51] <kompewter> [ IIIF and DSpace - DSpace - DuraSpace Wiki ] - https://wiki.duraspace.org/display/DSPACE/IIIF+and+DSpace
[15:52] <tdonohue> Oh, yes. Sorry, I should have asked about other topics here. Let's pause for a moment (we can do PR reviews in a bit at the top of the hour)
[15:53] <tdonohue> Glad to hear about the IIIF meeting. Have you talked to / invited bollini (or someone from 4Science)? My understanding is they (4Science) have IIIF integration they are wanting to get more eyes on & make open source
[15:54] <terry-b> Andrea indicated that he will attend (via Slack). The wiki attendees are incomplete
[15:54] <tdonohue> Ok, glad to hear!
[15:55] <tdonohue> Just wanted to be sure you all had touched base with him...as it sounds like this effort is farther along then I initially thought ;)
[15:55] <terry-b> His prototype is nice and it seems to address multiple use cases.
[15:56] <tdonohue> Oh, and here's more on 4Science's DSpace Add-ons. My understanding is they want to eventually open source them all, but they need to make back some $$ to support the building of the add-ons: http://www.4science.it/en/dspace-add-ons/
[15:56] <kompewter> [ DSpace add-ons | 4Science ] - http://www.4science.it/en/dspace-add-ons/
[15:56] <tdonohue> Their IIIF image viewer is here: http://www.4science.it/en/iiif-image-viewer/
[15:56] <kompewter> [ IIIF Image Viewer | 4Science ] - http://www.4science.it/en/iiif-image-viewer/
[15:57] <tdonohue> (and I see that Andrea linked to that in the comments on that wiki page already... I should have looked closer to begin with)
[15:57] <tdonohue> Are their any other announcements or notes to add here (before the meeting closes & we move into Backlog Hour)?
[15:57] <tdonohue> s/their/there/
[15:57] <kompewter> tdonohue meant to say: Are there any other announcements or notes to add here (before the meeting closes & we move into Backlog Hour)?
[15:57] <hpottinger> we should bountysource those addons :-)
[15:59] <hpottinger> https://www.bountysource.com/
[15:59] <kompewter> [ Bountysource ] - https://www.bountysource.com/
[15:59] <terry-b> Will the backlog be here or on Slack?
[16:00] <terry-b> I will be lurking. I need to complete some testing,
[16:00] * jcreel (~jcreel@jcreel.tamu.edu) Quit (Quit: jcreel)
[16:01] <tdonohue> As we are now at the top of the hour, we'll close up the meeting for today.
[16:02] <tdonohue> I'd vote we do the Backlog Review Hour here (if everyone is OK with that)...that way it's logged and others can catch up later on. So, we can just transition this into Backlog Hour
[16:03] <tdonohue> Who here is sticking around for Backlog Hour?
[16:03] <mhwood> I'm here.
[16:05] <tdonohue> Ok, well, I'm hoping others are lurking ;) I'd still like to go through these "quick wins"
[16:05] <tdonohue> Here's that list again: https://github.com/DSpace/DSpace/pulls?q=is%3Apr+is%3Aopen+label%3A%22quick+win%22
[16:05] <kompewter> [ Pull Requests · DSpace/DSpace · GitHub ] - https://github.com/DSpace/DSpace/pulls?q=is%3Apr+is%3Aopen+label%3A%22quick+win%22
[16:06] <tdonohue> The first two (1715 and 1714) were discussed in the DevMtg (see notes above)
[16:06] <tdonohue> So, skipping those here
[16:07] <tdonohue> Next is DSPR#1713 / DSPR#1712 (same fix on different branches)
[16:07] <kompewter> [ https://github.com/DSpace/DSpace/pull/1713 ] - DS-3573: Filtername in XMLUI Discovery filter labels by YanaDP ¡ Pull Request #1713 ¡ DSpace/DSpace ¡ GitHub
[16:07] <kompewter> [ https://github.com/DSpace/DSpace/pull/1712 ] - DS-3573: Filtername in XMLUI Discovery filter labels by YanaDP ¡ Pull Request #1712 ¡ DSpace/DSpace ¡ GitHub
[16:08] <tdonohue> I gave these both a quick review. Nothing obviously wrong in the code... I didn't test them, but as the changes are small, I'm trusting Atmire folks did test these ;)
[16:09] <hpottinger> lurking, sorry, also multitasking, I have a full meeting docket this week, and a big TODO list as well
[16:09] <tdonohue> thoughts on 1712/1713? Do we just merge these minor fixes to Mirage2?
[16:10] <mhwood> *sigh* I don't even know what language a .hbs is.
[16:10] <hpottinger> handlebars?
[16:10] <tdonohue> hbs ="handlebars"
[16:10] <mhwood> Makes sense, thanks.
[16:10] * ntorres (c1895861@gateway/web/freenode/ip. Quit (Ping timeout: 260 seconds)
[16:10] <hpottinger> I think they are a javascript thing?
[16:10] <mhwood> It's a templating system of some sort.
[16:11] * hpottinger has a vacant look.
[16:11] <tdonohue> it's JS templates: http://handlebarsjs.com/
[16:11] <kompewter> [ Handlebars.js: Minimal Templating on Steroids ] - http://handlebarsjs.com/
[16:11] <hpottinger> oh, we were both right! :-)
[16:11] <tdonohue> And yes, the Mirage2 theme uses it a bit
[16:12] <hpottinger> so, 1713 needs a test?
[16:13] <hpottinger> btw, I've got another meeting in 15 minutes, I'll stack it on this one, so... in 15 minutes I will be more distracted than normal, I appologize in advance.
[16:14] <tdonohue> If you are willing to give it a quick test, sure. I think it seems relatively obvious...and you can see this behavior in Mirage2 on http://demo.dspace.org
[16:14] <kompewter> [ DSpace 6.0 Demonstration Repository ] - http://demo.dspace.org
[16:14] <tdonohue> Just do a search from http://demo.dspace.org/xmlui/ ... then add a filter to that search. You'll see the filter's *value* but not the name of the filter (which is slightly confusing)
[16:15] <kompewter> [ DSpace Home ] - http://demo.dspace.org/xmlui/
[16:15] <tdonohue> I.e. if you add a filter on Title="test", then the filter shows up as just "test"...when it likely should say "title:test" (or similar)
[16:16] <mhwood> And that seems to be what the patch does: "title: test"
[16:16] <tdonohue> So, honestly, I was asking whether this is a "just merge it". The bug seems obvious, and the fix seems reasonable. I'm assuming this was tested as it's coming from Atmire & Bram approved it
[16:16] <tdonohue> mhwood: yes, exactly
[16:16] <mhwood> Just merge it.
[16:16] <hpottinger> I'm looking at a search on demo
[16:17] <hpottinger> where would this text appear?
[16:17] <tdonohue> Search & then apply a Filter. The text appears under the searchbox as a little filter
[16:17] <hpottinger> oh... is the form not enough feedback?
[16:18] <tdonohue> Once you *apply* the filter, the form (for the filter) disappears
[16:18] <tdonohue> And then all you see is a little "label" like thing
[16:18] <hpottinger> aha
[16:18] <hpottinger> +1 merge taht
[16:19] <tdonohue> Ok, merging them
[16:20] <tdonohue> done
[16:20] <hpottinger> ᕦ(ᐛ)ᕤ
[16:21] <tdonohue> next in that list is DSPR#1709, which terry-b already volunteered to test. So, once tested, I'll get it merged
[16:21] <kompewter> [ https://github.com/DSpace/DSpace/pull/1709 ] - DS-3516 ImageMagick PDF Thumbnail class should only process PDFs by alanorth ¡ Pull Request #1709 ¡ DSpace/DSpace ¡ GitHub
[16:21] <tdonohue> next is DSPR#1708
[16:21] <kompewter> [ https://github.com/DSpace/DSpace/pull/1708 ] - DS-3568. UTF-8 characters are now supported in configuration files by christian-scheible ¡ Pull Request #1708 ¡ DSpace/DSpace ¡ GitHub
[16:21] <tdonohue> I approved this code...but needs a test. mhwood volunteered to test it
[16:22] <mhwood> On my list.
[16:22] <tdonohue> It *looks* obvious to me, but just requires quick test
[16:22] <tdonohue> thanks mhwood
[16:22] <hpottinger> I put it on my "in case I'm bored" list
[16:22] <tdonohue> Next is DSPR#1707, which hpottinger volunteered to test (earlier today)...so, we'll skip that too
[16:22] <kompewter> [ https://github.com/DSpace/DSpace/pull/1707 ] - DS 3558 Case-insensitive bot matching option by Frederic-Atmire
[16:22] <tdonohue> After that is DSPR#1703
[16:22] <kompewter> [ https://github.com/DSpace/DSpace/pull/1703 ] - DS-3553: when creating a new version, do context complete before redirecting to the submission page by samuelcambien
[16:23] <tdonohue> aha. this one. This has gone through several iterations (as I worked with samuelcambien on improving it). It's now a simple one-liner
[16:23] <hpottinger> does that even need testing?
[16:24] <tdonohue> And it's now obvious to me...as it parallels other methods in the same JS file (whereas previously it was missing this context.complete())
[16:24] <tdonohue> So, no, I think this is likely now a "just merge it". I had simply forgotten about this one
[16:25] <tdonohue> do others agree?
[16:25] <mhwood> Yes.
[16:26] <hpottinger> +1
[16:27] <tdonohue> I just added my approval & will merge it
[16:27] <tdonohue> merged & closed ticket
[16:28] <tdonohue> next up, DSPR#1699 / DS-3554
[16:28] <kompewter> [ https://jira.duraspace.org/browse/DS-3554 ] - [DS-3554] Breaks at Subissions page when some submission&#39;s title is empty (null) - DuraSpace JIRA
[16:28] <kompewter> [ https://github.com/DSpace/DSpace/pull/1699 ] - Fix for DS-3554 by enrique ¡ Pull Request #1699 ¡ DSpace/DSpace ¡ GitHub
[16:28] <tdonohue> Oh, another one that might be another "Just merge it". It's a one-liner with some code realignment around it
[16:28] <mhwood> Ah, one of those mysterious "Fix for Ds-xxxx" PRs.
[16:29] <tdonohue> Yes, probably needs a better title. ;)
[16:29] <tdonohue> Do others agree we should just merge this one?
[16:29] <mhwood> Yes.
[16:30] <hpottinger> hard to see what the actual change is
[16:30] <mhwood> line 362
[16:30] <tdonohue> It should be highlighted by Github ;)
[16:31] <tdonohue> Yes it's a change of "title.length() > 0" to "StringUtils.isNotBlank(title)"
[16:31] <hpottinger> ah, OK... merge taht
[16:32] <hpottinger> second time I've made that typo during this meeting, might need more tea
[16:32] <mhwood> IRC needs mod_speling.
[16:32] <tdonohue> merged & closed PR
[16:32] * PhilipV (~philip@ has left #duraspace
[16:33] <tdonohue> next up is DSPR#1697 / DSPR#1696 (same fix multiple branches)
[16:33] <kompewter> [ https://github.com/DSpace/DSpace/pull/1697 ] - DS-2748: Do not throw an exception in the PageNotFoundTransformer by tomdesair
[16:33] <kompewter> [ https://github.com/DSpace/DSpace/pull/1696 ] - DS-2748: Do not throw an exception in the PageNotFoundTransformer by tomdesair ¡ Pull Request #1696 ¡ DSpace/DSpace ¡ GitHub
[16:34] <tdonohue> DS-2748 is the problem
[16:34] <kompewter> [ https://jira.duraspace.org/browse/DS-2748 ] - [DS-2748] Cocoon logs overly verbose when 404s occur - DuraSpace JIRA
[16:34] <tdonohue> This seems tiny...but it might be worth a quick sanity check here?
[16:35] <hpottinger> we shouldn't be caching 404s
[16:35] <mhwood> That's the "prevent Cocoon leaking memory" bit.
[16:36] <hpottinger> is this the "bot hit me too hard" culprit?
[16:36] <mhwood> The rest is simply setting 404 on the response instead of throwing. Which makes much sense to me.
[16:37] <hpottinger> I like this change and I want it merged, does it need testing?
[16:37] <tdonohue> Yes, this seems pretty obvious to me. We shouldn't be throwing a "Page cannot be found" error, but instead should return a 404 response. Not sure if it's worth testing though to ensure there's no side effects here?
[16:38] <tdonohue> I guess I'm *assuming* Cocoon will pass along this 404 response to the browser and not do something weird here
[16:38] <mhwood> Good point.
[16:38] <hpottinger> this will be very easy to test, I'll test it
[16:38] <tdonohue> So, I think I'd prefer a sanity test....just apply this, and make sure the browser sees 404's appropriately. If so, it sounds good to go
[16:39] <tdonohue> thanks hpottinger. will assign PR to you then
[16:39] <tdonohue> I'll give my +1 but add that it needs testing
[16:39] <mhwood> If Cocoon zaps the status, we can still remove the caching.
[16:40] <hpottinger> assign them both, and Cocoon better do what it's told
[16:40] * hpottinger looks sternly at Cocoon.
[16:41] <mhwood> Cocoon shrugs: "200 looks better...."
[16:41] * hpottinger tries to look even more stern.
[16:41] <tdonohue> I've added my +1 to both along with a comment that we need to test that Cocoon respects these 404 responses
[16:42] <tdonohue> Cocoon does annoyingly love to return 200. "Hey, I cannot find what you want, but here's a 200 response to hopefully make you happy!"
[16:42] <hpottinger> ¯\_(ツ)/¯
[16:43] <tdonohue> Ok, next in the list... DSPR#1682 / DS-3535
[16:43] <kompewter> [ https://jira.duraspace.org/browse/DS-3535 ] - [DS-3535] Reduced error logging by interrupted download - DuraSpace JIRA
[16:43] <kompewter> [ https://github.com/DSpace/DSpace/pull/1682 ] - [DS-3535] Reduced error logging by interrupted download by pbroman
[16:43] <tdonohue> Oh and hey... pnbecker approved this, and I did too. I guess I should've merged this. (Not sure why I didn't)
[16:43] <hpottinger> OK, I've run off to my standup
[16:45] <tdonohue> merged it
[16:45] <mhwood> If some IOException unwinds all the way up to DSpaceServlet without logging, we need to fix that where we know what went wrong.
[16:46] <tdonohue> yes, agreed. But, as JSPUI time is limited, I think this fix looks good enough
[16:46] <tdonohue> next in our list is DSPR#1672 / DS-3522
[16:46] <kompewter> [ https://jira.duraspace.org/browse/DS-3522 ] - [DS-3522] Submission Resource Policy not correctly removed during XMLWorkflow - DuraSpace JIRA
[16:46] <kompewter> [ https://github.com/DSpace/DSpace/pull/1672 ] - DS-3522 Ensure Submission Policies are removed in XMLWorkflow by KingKrimmson
[16:48] <tdonohue> This is a bit of code cleanup... it seems we are creating extra ResourcePolicies... they aren't harmful, but they are not useful either
[16:49] <tdonohue> Hmm...though it sounds like we need to see whether this fix is also needed in 6.x (per ticket comments)
[16:50] <mhwood> More like: the policies are not the right type, so they don't get cleaned up.
[16:50] <tdonohue> yes, true
[16:51] <tdonohue> This seems rather obvious to me. But, we should see if this needs "porting" to 6.x as well (my guess is that's likely, but would take a tad more effort as XmlWorkflowManager was replaced by XmlWorkflowService)
[16:51] <mhwood> Yes
[16:51] <mhwood> Also to trunk
[16:52] <tdonohue> yes
[16:53] <tdonohue> I'll add a comment to the ticket that we need a 6.x version (which likely could be cherry-picked to master)
[16:53] <mhwood> Thanks.
[16:53] <hpottinger> "trunk"
[16:53] <mhwood> Sorry, old SVN habit kicked in.
[16:53] <hpottinger> I'm just kidding ya
[16:53] <mhwood> :-)
[16:54] <tdonohue> I'm going to stop there for today...as I want to add in this comment, and it'll probably take me a few minutes anyhow ;)
[16:55] <tdonohue> So, thanks for the help here mhwood & hpottinger! You and others are also free to go through more "quick wins" whenever you find extra time (I know that can be rare though)
[16:57] <mhwood> OK, thanks for leading.
[16:59] <hpottinger> thanks! I'm going to head to lunch now, will try to finish up some dev work for local projects, and then get some DSpace pull request testing in
[17:03] * hpottinger (~hpottinge@ Quit (Quit: Leaving 三三ᕕ( ᐛ )ᕗ LATER TATERS!)
[17:04] * terry-b (~chrome@75-172-43-243.tukw.qwest.net) Quit (Remote host closed the connection)
[17:52] * tdonohue (~tdonohue@dspace/tdonohue) has left #duraspace
[17:52] * tdonohue (~tdonohue@dspace/tdonohue) has joined #duraspace
[17:54] * tdonohue (~tdonohue@dspace/tdonohue) has left #duraspace
[18:37] * dyelar (~dyelar@biolinux.mrb.ku.edu) has joined #duraspace
[20:03] * hpottinger (~hpottinge@ has joined #duraspace
[20:44] * DSpaceSlackBot (~tdonohue@dspace/tdonohue) has joined #duraspace
[20:45] * DSpaceSlackBot is now known as tdonohue
[20:57] * DSpaceSlackBot1 (~DSpaceSla@ec2-50-17-201-82.compute-1.amazonaws.com) has joined #duraspace
[20:57] * tdonohue (~tdonohue@dspace/tdonohue) has left #duraspace
[20:59] * mhwood (mwood@mhw.ulib.iupui.edu) has left #duraspace
[21:04] * DSpaceSlackBot1 (~DSpaceSla@ec2-50-17-201-82.compute-1.amazonaws.com) Quit (Remote host closed the connection)
[21:04] * tdonohue (~tdonohue@dspace/tdonohue) has joined #duraspace
[21:04] * DSpaceSlackBot (~DSpaceSla@ec2-50-17-201-82.compute-1.amazonaws.com) has joined #duraspace
[21:06] <tdonohue> Testing if Slack<->IRC integration now works again. This message was posted in #duraspace IRC.
[21:06] <DSpaceSlackBot> <tdonohue> And this one was posted in dev-mtg Slack channel
[21:06] <tdonohue> Works again!
[21:09] * DSpaceSlackBot (~DSpaceSla@ec2-50-17-201-82.compute-1.amazonaws.com) Quit (Remote host closed the connection)
[21:10] * DSpaceSlackBot (~DSpaceSla@ec2-50-17-201-82.compute-1.amazonaws.com) has joined #duraspace
[21:44] * tdonohue (~tdonohue@dspace/tdonohue) has left #duraspace
[21:59] * hpottinger (~hpottinge@ Quit (Quit: Leaving 三三ᕕ( ᐛ )ᕗ LATER TATERS!)

These logs were automatically created by DuraLogBot on irc.freenode.net using the Java IRC LogBot.