Ticket #6457 (closed: fixed)
Improve ICAT Searching
Reported by: | Owen Arnold | Owned by: | Jay Rainey |
---|---|---|---|
Priority: | critical | Milestone: | Release 3.0 |
Component: | Framework | Keywords: | ICAT |
Cc: | Blocked By: | ||
Blocking: | #1807, #3827, #6441, #8202, #8207, #8209, #8218 | Tester: | Samuel Jackson |
Description (last modified by Nick Draper) (diff)
The following comments were made during unscripted testing of #6400 prior to release 2.4 of mantid
While the catalogue searching "Works", the default values set for the fields in Mantid are very annoying. The interface does not feel logical. For example, the run start and run end numbers seem both to default to 0. Therefore I get zero results back when doing a keyword search, because It's using hidden values, which I did not set and cannot see. I'm sure this is at the bottom priority of things to fix, but It might make excellent work for the next set of students we get.
I've created a ticket out of this comment and assigned it to N. Draper to ensure that we still have knowledge of this at the time new students arrive.
Change History
comment:4 Changed 7 years ago by Nick Draper
- Milestone changed from Release 2.6 to Backlog
Moved to backlog at the code freeze for R2.6
comment:5 Changed 7 years ago by Nick Draper
- Owner changed from Nick Draper to Jay Rainey
- Description modified (diff)
- Milestone changed from Backlog to Release 3.0
Here's some ideas of things to improve
comment:6 Changed 7 years ago by Jay Rainey
This ticket has been resolved in #7639. To verify this is true:
- Open Mantid
- Click Catalog -> Login (Located in the top menu bar).
- Click My Data search. You should be able to view your specific data.
- Click Basic search and ensure all fields work correctly.
- Type bob into the Keywords field.
- Results are returned.
Similar functionality has been implemented for ICat4. To test this on ICat4 you will need to change attributes in the Facilities.xml file. To do this, read comment:25 in my other ticket.
comment:7 Changed 7 years ago by Jay Rainey
- Status changed from new to verify
- Resolution set to fixed
comment:8 Changed 7 years ago by Keith Brown
- Status changed from verify to verifying
- Tester set to Keith Brown
If this was fixed in another ticket, the resolution should be "invalid" But i'll test it anyway
comment:9 Changed 7 years ago by Keith Brown
Again, you've not pushed this to develop i can't start the test
comment:10 Changed 7 years ago by Keith Brown
- Status changed from verifying to reopened
- Resolution fixed deleted
The date doesn't work as i would expect. In order to search for a result of a specific day (so from 00:00:00 to 23::59:59 on a single day) I would expect to insert an identical start and end date. but this doesn't happen, instead I'm given no results.
I'd like to be able to use the same date in both fields.
comment:11 Changed 7 years ago by Jay Rainey
- Status changed from reopened to inprogress
Now produces the correct datetime from a given string. Refs #6457.
Changeset: dedb47e80aef0703e063bd1018f59a0e9a88c7f6
comment:12 Changed 7 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
The problem occurs in the getTimevalue method in CatalogSearchParam.cpp, which does not produce the correct date/time when the user selects a date in the calendar. For example, if you select today, it will give you yesterday. This will be taken care of during maintenance, and will be addressed in #8152.
As a fix for the release I have added the hours/days to the start/end date in order to allow the user to select the correct day, and also allow them to select a specific day (as suggested by Keith above).
To test
This functionality only works on ICAT4. In order to test this you will need to change the Facilities.xml to:
<catalog name="ICat4Catalog"> <soapendpoint url="https://icatisis.esc.rl.ac.uk/ICATService/ICAT"></soapendpoint>
To verify that the start/end date produced the incorrect results open Mantid prior to testing and:
- Log into the catalog.
- Right click the Results log and set the Log level to Debug. This will allow you to view the search query that contains the start/end date in ICAT4.
- Load Basic search, and Select a start and end date.
- Click Search and Look at the log results. Note how the start/end date are 1 day short of the date you selected, and both times are 23:00:00.
To verify I have addressed the problem obtain my branch/code changes and run through the steps above. The start/end date should now be the correct date. The start time should be: 00:00:00 while the end time should be: 23:59:59.
comment:14 Changed 7 years ago by Keith Brown
- Status changed from verifying to verify
- Tester Keith Brown deleted
I seem to get a fatal crash when I've change the xml file from ICAT3 to ICAT4, restart mantid, then try and search.
returning to pool to make sure it's not just my environment
comment:15 Changed 7 years ago by Nick Draper
- Status changed from verify to verifying
- Tester set to Nick Draper
comment:16 Changed 7 years ago by Nick Draper
- Status changed from verifying to reopened
- Resolution fixed deleted
Still crashes on windows, heres the call stack
msvcr110d.dll!_Strftime_l(char * string, unsigned __int64 maxsize, const char * format, const tm * timeptr, void * lc_time_arg, localeinfo_struct * plocinfo) Line 285 C++ msvcr110d.dll!strftime(char * string, unsigned __int64 maxsize, const char * format, const tm * timeptr) Line 190 C++ MantidKernel.dll!Mantid::Kernel::DateAndTime::toFormattedString(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > format) Line 531 C++ MantidICat.dll!Mantid::ICat::ICat4Catalog::formatDateTime(__int64 timestamp) Line 714 C++ > MantidICat.dll!Mantid::ICat::ICat4Catalog::getSearchQuery(const Mantid::ICat::CatalogSearchParam & inputs) Line 128 C++ MantidICat.dll!Mantid::ICat::ICat4Catalog::search(const Mantid::ICat::CatalogSearchParam & inputs, boost::shared_ptr<Mantid::API::ITableWorkspace> & outputws) Line 251 C++ MantidICat.dll!Mantid::ICat::CatalogSearch::exec() Line 99 C++ MantidAPI.dll!Mantid::API::Algorithm::execute() Line 552 C++ MantidAPI.dll!Mantid::API::Algorithm::executeAsyncImpl(const Poco::Void & __formal) Line 1337 C++ MantidAPI.dll!Mantid::API::AlgorithmProxy::executeAsyncImpl(const Poco::Void & dummy) Line 116 C++ MantidAPI.dll!Poco::ActiveRunnable<bool,Poco::Void,Mantid::API::AlgorithmProxy>::run() Line 85 C++ PocoFoundation64d.dll!Poco::PooledThread::run() Line 215 C++ PocoFoundation64d.dll!Poco::ThreadImpl::runnableEntry(void * pThread) Line 245 C++ kernel32.dll!BaseThreadInitThunk () Unknown ntdll.dll!RtlUserThreadStart () Unknown
comment:18 Changed 7 years ago by Owen Arnold
- Blocked By 7637 removed
(In #7637) There is a crash fix that must be sorted out. We also want to be confident that we can specify mac-friendly mount points.
comment:21 Changed 7 years ago by Jay Rainey
- Blocking 8209 added
(In #8209) To verify that this now works correctly log into the catalog, and perform a search. The result table will be populated, and the end date is never larger than the start date, and that they generally make sense..
To check this is true you could log into the ICAT4 web-gui.
comment:24 Changed 7 years ago by Jay Rainey
- Blocking 8202 added
(In #8202) To test this ticket you must have access to the archives.
- Enable debugging in the Results log window.
- Open the catalog and perform a search.
- Select an investigation to the Datafile table will be populated.
- Select a datafile to download, and note the output in the debug console. You will see the path before the transformation (e.g. the windows path), and after (e.g. containing your mounted archive point.
comment:26 Changed 7 years ago by Jay Rainey
- Blocking 6441 added
(In #6441) To verify that my changes have addressed the above issues open the ICAT GUI, and check that what I said above is valid.
comment:29 Changed 7 years ago by Jay Rainey
Microsoft do not support %F %T time format. Refs #6457.
Changeset: 2cb26c2b99624a20fc07060f3eefab261220f6cb
comment:30 Changed 7 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
To verify that my changes fixed the problem of ICAT crashing on windows download an older version of ICAT from jenkins when the crash occurred (to verify it did), and then a newer version ( > 2542 on W7) when my changes have been introduced.
Currently access to ICAT4 is not functioning correctly. However, this ticket can still be tested (icat4 end-point on develop is invalid, but the catalog is set to ICAT4, which is where the changes are).
To verify:
- Open Mantid, and click "Search".
- Press "Cancel" to login. The ICAT GUI will open.
- Perform a search. It should no longer crash.
comment:31 Changed 7 years ago by Jay Rainey
- Status changed from verify to reopened
- Resolution fixed deleted
comment:32 Changed 7 years ago by Jay Rainey
- Status changed from reopened to verify
- Resolution set to fixed
comment:33 Changed 7 years ago by Jay Rainey
- Status changed from verify to reopened
- Resolution fixed deleted
comment:34 Changed 7 years ago by Jay Rainey
The above fix fixed one crash (related to dates), but when I perform a search that produces results I get another crash. Clearly something is going wrong when populating the table. Looking into it now.
comment:35 Changed 7 years ago by Jay Rainey
- Status changed from reopened to inprogress
Merge branch 'feature/8126_icat4_external_download' into develop. Refs #6457.
Conflicts:
Code/Mantid/Framework/ICat/src/ICat4/ICat4Catalog.cpp
Changeset: b7b8705cb8b3bcaa6a05c9eab9096a14ece0c5f0
comment:36 Changed 7 years ago by Jay Rainey
I accidentally committed to the wrong ticket a fix for the crash noted in comment:34. The above merge to develop was in fact made in #8126.
To rectify this I will download and test a develop build (since it has changes in #8126 that are required to see the crash), and if the above change fixes it then #8126 will need to be tested before this ticket.
comment:39 Changed 7 years ago by Jay Rainey
- Blocking 8126 removed
comment:39 Changed 7 years ago by Jay Rainey
I am going to resolve the windows crash in #8126. To test this ticket see comment:1, and verify that my solution in comment:29 addresses Keith's suggestion in comment:10
comment:40 Changed 7 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
- Blocking 1807, 8202 added
comment:41 Changed 7 years ago by Samuel Jackson
- Status changed from verify to verifying
- Tester set to Samuel Jackson
comment:47 Changed 7 years ago by Samuel Jackson
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/6457_fix_icat_win_crash'
Full changeset: beb37cd6d390c9857dfa68ee10407cf7ab9f5312
comment:48 Changed 7 years ago by Samuel Jackson
The interface still requires a lot of work but as I can see these issues will be handled in other tickets and the fixed version of the query is showing up I'll mark this as fixed.
comment:49 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 7303
Moved to r2.6 at the end of r2.5