feat(robot-server): show run time parameters in pending analysis

[sanni-t]

Closes AUTH-282

Overview

Follow-up to #15217

This PR changes a pending analysis' AnalysisSummary to include the run time parameters (with overrides).
In order to do this, we will now be parsing the RTPs before analysis response is created. This is a very quick process and shouldn't cause any perceptible delays in the response of neither POST /protocols nor POST /runs/.../analyses endpoints, as long as recommended python protocol guidelines are followed.

Test Plan

• Upload a protocol with run time parameters, verify that the response contains run time parameter definitions along with the values specified in the request
• Send a POST request on /protocols/{protocolId}/analyses endpoint with different RTP values, or by specifying the forceReanalysis flag, and verify that the newly pending analysis also contains (updated) RTPs
• Verify that pending summaries in GET endpoints also contain RTPs
• Verify that summaries of completed analyses don't expose RTPs, while RTPs are available in the full analyses.

Changelog

• Main change is the introduction of AnalysesManager. The analyses manager is responsible for creating a protocol analyzer per analysis, extracting the run time parameters from the runner and starting the analysis.
• To facilitate this extraction of run time parameters, the ProtocolAnalyzer has been divided into two functions- one that loads the runner and other that runs the runner.
• Updated the router to use AnalysesManager for starting analyses instead of directly interacting with ProtocolAnalyzer.

Review requests

• Does the new architecture of AnalysesManager & updated ProtocolAnalyzer makes sense.
• This PR doesn't expose the run time parameters in analysis summaries of completed analyses because of two reasons- (1) as opposed to the RTPs of a pending analysis, the completed analysis isn't guaranteed to be cached in memory. So we would need to read the completed analysis from database, de-serialize it and then return it. This would be made easier when we start storing RTPs in their own database table, but we're not there yet. (2) I'm not sure if it is needed. A completed analysis already contains the RTPs.
  Would like to get others' opinions on this.
• Usual code review

Risk assessment

• Medium. It changes a core part of protocol analysis but the end effect is a small change to the API. As long as tests are passing, it shouldn't cause any problems.
• Small risk of analysis behavior change for badly written protocols as mentioned in this comment.

[SyntaxColoring]

Thank you! I know this is all a work in progress, but some suggestions so far:

[SyntaxColoring]

I think we want to catch Exception instead of BaseException. As a heuristic: if something raises KeyboardInterrupt or asyncio.CancelledError, we do not want to swallow it.

I see that the ProtocolAnalyzer internals are also currently catching BaseException. I think those should be catching Exception, too.

[jbleon95]

Changes look good and make sense to me, and will make adding the "new file needed" analysis result much easier

[SyntaxColoring]

yay ty

[sanni-t]

from discussion w/ @SyntaxColoring: making runner loading as a part of the analyzer initialization will help prevent misuse of load_runner(), for example, calling load_runner() multiple times.

I like this suggestion. Will take it into account when making the upcoming changes to the analysis arch.