Ticket #11067 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Refactor SNS & ISIS data archive implementations to use InternetHelper

Reported by: Nick Draper Owned by: Nick Draper
Priority: major Milestone: Release 3.4
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Pete Peterson

Description (last modified by Nick Draper) (diff)

The SNS & ISIS data archive implementations use poco net classes directly, move them over to using the InternetHelper class

Change History

comment:1 Changed 6 years ago by Nick Draper

  • Description modified (diff)
  • Summary changed from Refactor ICAT interface calls to use InternetHelper (cloned) to Refactor SNS & ISIS data archive implementations to use InternetHelper

comment:2 Changed 6 years ago by Nick Draper

re #11067 refactor isis&sns data archives to use internethelper

Changeset: 4db79ac4a011fbae58108b01e4476c14632b0e06

comment:3 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:4 Changed 6 years ago by Nick Draper

  • Status changed from assigned to verify
  • Resolution set to fixed

Testing as ​https://github.com/mantidproject/mantid/pull/212

comment:5 Changed 6 years ago by Federico M Pouzols

  • Status changed from verify to verifying
  • Tester set to Federico M Pouzols

comment:6 Changed 6 years ago by Federico M Pouzols

The builds failed on linuxes, I think because of the uppercase NET in SNSDataArchive.cpp:18, "#include <Poco/NET/HTTPResponse.h>". It also looks like on osx an include is needed for std::stringstream, probably <sstream>

Other than that everything looks fine. I could do the testing at ISIS and then leave it for someone at SNS to finish it.

comment:7 Changed 6 years ago by Nick Draper

re #11067 moved http status codes to internethelper class

Changeset: 779c1b147d49154d7834d55b8884b55f1e99ce95

comment:8 Changed 6 years ago by Nick Draper

re #11067 another stringstream to ostringstream

Changeset: 4986eafcbe2ebeff1a1fe2aa6b2c399e9c48254a

comment:9 Changed 6 years ago by Federico M Pouzols

This works well here at ISIS (tested on win7). I'm not merging it as it still needs a check from someone at SNS. Just a note for other testers in case you run into the same issue: I started testing on a debian virtual machine inside the win7 machine, and for some reason it doesn't work well (the request is correctly sent through our proxy but at some point packets are being dropped).

Unassigning myself now. Please could someone at SNS give it a check?

comment:10 Changed 6 years ago by Pete Peterson

  • Tester changed from Federico M Pouzols to Pete Peterson

comment:11 Changed 6 years ago by Pete Peterson

This cannot be cleanly merged into master. Please merge master into your branch and we can try again.

comment:12 Changed 6 years ago by Nick Draper

re #11067 merge in master and fix conflict

Changeset: 21f95fb5010bacbcc53c04e3fe58e52bf3774b89

comment:13 Changed 6 years ago by Federico M Pouzols

Jenkins, retest this please...

comment:14 Changed 6 years ago by Federico M Pouzols

Jenkins, don't lose hope and retest this please

comment:15 Changed 6 years ago by Pete Peterson

  • Status changed from verifying to closed

Merge pull request #212 from mantidproject/feature/11067_DataArchive_using_Internethelper

Refactor isis & sns data archives to use internethelper

Full changeset: ​e48db368939db02c0f1c2682db8393f2f04ac534

comment:16 Changed 6 years ago by Federico Montesino Pouzols

fix param name, doxygen warning, comes from re #11067

Changeset: 40226182d32c0d58ecdd9dd2618915d74e7312c0

comment:17 Changed 6 years ago by Federico Montesino Pouzols

fix param name, doxygen warning, comes from re #11067

Changeset: a6291b9742d85332952f3bf33714bb6f04b6ac1b

comment:18 Changed 5 years ago by Nick Draper

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

comment:19 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​11906

Note: See TracTickets for help on using tickets.