[DS-1272] Setting discovery to be the default search/browse engine #91

Merged
merged 1 commit into from Oct 8, 2012

Projects

None yet

5 participants

@KevinVdV
Owner
KevinVdV commented Oct 4, 2012

No description provided.

Owner
abollini commented Oct 4, 2012

It looks as the pull request set discovery as default search/browse provider only for the XMLUI and not JSPUI, is this the intent? with the current configuration there is some overhead as the browse and search (lucene) consumer are also enabled by default. IMHO it will be better to enable discovery for both UIs or keep it optional

Owner
helix84 commented Oct 4, 2012

I disagree. XMLUI Discovery was introduced in 1.7 and only becomes default now. I'd leave some time for JSPUI Discovery to grow out of any eventual childhood diseases (which will be discovered only when other people start using it) and make the switch with next release. Please, don't take it personally, I'm not implying that there are any known problems with Discovery and you surely did a great job with it. I just have the best interest of JSPUI users in mind.

Owner
tdonohue commented Oct 4, 2012

Just wanting to note that if we do not enable Discovery by default for both XMLUI & JSPUI, we will need some clear documentation around this.

As it is, our 3.x Discovery documentation already is missing any information about JSPUI + Discovery:
https://wiki.duraspace.org/display/DSDOC3x/Discovery

We obviously need to make it clear that Discovery now is also available for JSPUI, and provide instructions for how to enable or disable it for the JSPUI.

We also may want to look into whether there are ways to split up that Discovery documentation into a few sub-pages -- e.g. a generic "Discovery" page (for general shared info), and then perhaps separate "Discovery for XMLUI" and "Discovery for JSPUI" subpages which can describe how to enable/disable & customize Discovery for each interface.

Owner
mdiggory commented Oct 4, 2012

We can build out the documentation if the intent is strong to have it enabled in JSPUI by default. I might recommend that if we enable int he RC candidate, this would give sufficiently more time to evaluate and document it. If immense issues arise, then revert it back so that Search and Browse are default in JSPUI.

Note, I think that Withdrawn Items view is still Browse driven. So I'm ok with the idea of having all three consumers enabled by default for the time being, even if Discovery is only used in the User interfaces.

Owner
abollini commented Oct 5, 2012

I will prefer to have discovery enabled by default in 3.0 but if we do it we should really do it for both UI. Enable this only for XMLUI will make the configuration for jspui harder to do well for people that will use it with or without discovery. IMHO we should not enable by default all consumers providing an inefficient configuration for most of our users.
If we switch also to discovery for browsing, the browse consumer can be disabled for both xmlui and jspui (but the solrDAOs need to be configured). Withdrawn items are managed by solr browse.
In any case we should test jspui discovery and solr browse in testhaton.
Wait for 4.0 to enable discovery and solr browse as default for both UIs is still an option for me.
I will update the discovery documentation with a jspui section during the weekend

Owner
mdiggory commented Oct 5, 2012

Guys,

What we need to be clear about is that this is not about forcing everyone who is upgrading to DSpace 3.0 to have to use Discovery. If you are upgrading, your going to be doing some serious configuration and testing of the platform and you will enable what you want. This is for new DSpace setups and what the community wants to encourage for new users of the platform. Which in my opinion, is that they start to use the new technology and features, not the old stuff, that IMO, we should be trying to delete in 4.0. I know, this should probably be discussed in the larger community.

Andrea, given we have until wednesday, can you provide a patch that would enable Discovery in JSPUI and SolrBrowse by default?

Then we can then make it the case that testathon instances are really testing Discovery+ JSPUI throughly and any bugs can be cleared during the release candidate stage. This would be more proactive then saying that because its new, we shouldn't enable it by default.

Mark

Owner
abollini commented Oct 8, 2012

Hi Mark,
I have made a pull request on your branch to enable Discovery also for JSPUI and Browse. If you merge my pull request it will be automatically included in this one.
Again I'm +1 with enable discovery and solr browse as default if it is applied to both UIs
-1 to apply these only to one UI (this will make the configuration/documentation work to hard for people that want to use JSPUI)
0 to delay this change to the 4.0

Owner
mdiggory commented Oct 8, 2012

Andrea, your pull request located here has a bunch of older commits that are already in the master / branch. I'm not sure why.

atmire#2

Owner
mdiggory commented Oct 8, 2012

I think that because it looks like the final release will not happen until tomorrow. I will merge this pull request and try to include Andreas work as well. I will run a local test and dryRun to verify everything will still work.

Mark

@mdiggory mdiggory merged commit 235e1a1 into DSpace:master Oct 8, 2012
Owner
abollini commented Oct 8, 2012

i think that your branch was bot up-to-date. I have started from the master, merged your branch and after made some changes

Inviato da Samsung MobileMark Diggory notifications@github.com ha scritto:Andrea, your pull request located here has a bunch of older commits that are already in the master / branch. I'm not sure why.


Reply to this email directly or view it on GitHub:
#91 (comment)

Owner
mdiggory commented Oct 8, 2012

Ok, theres a larger problem. All the tests were run with a solr instance running. They fail in the default case because not only is there no solr server running, but the testEnvoronment is not properly replacing the URL in the configuration.

I'm rolling back these commits and we will need to address this after RC1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment