Ticket #6075 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Removing a facility from Facilities.xml file does not complete kill it

Reported by: Martyn Gigg Owned by: Federico M Pouzols
Priority: critical Milestone: Release 3.4
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Martyn Gigg

Description (last modified by Anders Markvardsen) (diff)

Currently if you remove a facility from the Facilities.xml file but not from the 'supported.facilities' and start Mantid it complains about the missing facility (This was seen on LET that didn't have LENS in a different Facilties file it was using).

The error comes from MultiFileNameParser.cpp:212 where it uses the supported.facilities to iterate over facilities but doesn't find the one that has been taken out of the XML file.

The 'supported.facilities' property predates the Facilties.xml file and needs removing as it is now only a source of duplicate information. We can get everything from the Facilities file.

We need to make sure we find all of the places in the code that use the property and update them

Change History

comment:1 Changed 8 years ago by Nick Draper

  • Owner set to Karl Palmen
  • Status changed from new to assigned

comment:2 Changed 8 years ago by Karl Palmen

Find all "supported.facilities", Subfolders, Find Results 1, "Entire Solution", "*.c;*.cpp;*.h;"

C:\Mantid2\mantid\Code\Mantid\Framework\Kernel\src\MultiFileNameParser.cpp(202): std::string supportedFacilities = config.getString("supported.facilities");

C:\Mantid2\mantid\Code\Mantid\MantidPlot\src\ConfigDialog.cpp(680): QString property = QString::fromStdString(mantid_config.getString("supported.facilities"));

C:\Mantid2\mantid\Code\Mantid\MantidPlot\src\Mantid\FirstTimeSetup.cpp(24): QStringList faclist = QString::fromStdString(Mantid::Kernel::ConfigService::Instance().getString("supported.facilities")).split(";");

Matching lines: 3 Matching files: 3 Total files searched: 4584

comment:3 Changed 8 years ago by Karl Palmen

  • Milestone changed from Release 2.4 to Release 2.5

comment:4 Changed 7 years ago by Karl Palmen

  • Milestone changed from Release 2.5 to Release 2.6

comment:5 Changed 7 years ago by Nick Draper

  • Status changed from assigned to new

comment:6 Changed 7 years ago by Nick Draper

  • Component changed from Mantid to Framework

comment:7 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.6 to Backlog

Moved to the Backlog after R2.6

comment:8 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move to assigned at the introduction of the triage step

comment:9 Changed 6 years ago by Anders Markvardsen

  • Owner changed from Karl Palmen to Federico M Pouzols
  • Description modified (diff)

comment:10 Changed 6 years ago by Federico M Pouzols

  • Status changed from assigned to verify
  • Resolution set to invalid
  • Milestone changed from Backlog to Release 3.4

It seems that this ticket has become invalid. Changes elsewhere have wiped out the old 'supported.facilities'. Any use or iteration through the facilities that I could find in current master is based on the ConfigService getFacilityNames() or getFacilities() methods which give the list of facilities found in the facilities xml file and ignores the old 'supported.facilities' option. This includes ConfigDialog.cpp, MultiFileNameParser.cpp, FirstTimeSetup.cpp, and also InstrumentSelector.cpp, CatalogLogin.cpp, some tests like ConfigServiceTest, etc.

I couldn't find any traces of the old supported.facilities. Also, it is never mentioned in Properties_File.rst, Facilities_File.rst, etc. So I'd say there's nothing to do here.

To test:

  • double check that supported.facilities is never used in the code
  • you can also try to set 'supported.facilities' in Mantid.user.properties, it should be completely ignored.

comment:11 Changed 6 years ago by Martyn Gigg

  • Status changed from verify to verifying
  • Tester set to Martyn Gigg

comment:12 Changed 6 years ago by Martyn Gigg

  • Status changed from verifying to closed

Agreed. The clean up has already happened over time.

comment:13 Changed 5 years ago by Nick Draper

  • Resolution changed from invalid to fixed

Somehow these slipped through without a resolution. Set to Fixed.

comment:14 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6921

Note: See TracTickets for help on using tickets.