Skip to content

Commit

Permalink
fix(robot-server): fetching of data files used in runs of a protocol (#…
Browse files Browse the repository at this point in the history
…15908)

Closes AUTH-638

# Overview

Fixes a bug where CSV files used in protocol runs were not being saved
to the CSV RTP table for runs, and as a result, were not being included
in response for the `/protocols/{protocolId}/dataFiles` endpoint.

## Test Plan and Hands on Testing

Testing with app & ODD:
- [x] Upload a protocol that uses a CSV parameter
- [x] Send the protocol to a robot, upload the csv file to use
- [x] Run the protocol
- [x] Run the protocol again with a different CSV file -> Do this 5
times (so total 6 runs with 6 different CSV files). By re-running the
protocol 6 times, we are making the robot delete its oldest analysis
(since max analyses per protocol is 5), essentially deleting the first
CSV file from the *analysis* csv table, but not from runs table
- [x] Check that when you run the protocol again on the ODD, it shows
you all the 6 different CSV files previously uploaded

Testing with Postman/ direct HTTP requests:
- [x] Upload a few data files
- [x] Upload a protocol that uses a CSV parameter and specify a data
file (data_file_1) for the CSV param
- [x] Start a new analysis for the same protocol by specifying a second
data file (data_file_2) for the CSV param
- [x] Create a run for the protocol by specifying data_file_1 for its
CSV param
- [x] Create another run for the protocol by specifying a third data
file (data_file_3) for its CSV param
- [x] Check that the response to `GET /protocols/{protocolId}/dataFiles`
contains the 3 data files used with the runs & analyses. Check that they
are listed in the order that the files were uploaded to the server (via
`POST /dataFiles`)


## Changelog

- wired up CSV RTP table insertion during run creation
- updated the run deletion code to remove the CSV RTP entry from the
`run_csv_rtp_table` before deleting the run.
- updated the `../{protocolId}/dataFiles` response so that it lists the
files in the order they were uploaded.
- added tests

## Risk assessment

Low. Fixes bug
  • Loading branch information
sanni-t authored Aug 7, 2024
1 parent 927d368 commit 90127ff
Show file tree
Hide file tree
Showing 9 changed files with 322 additions and 47 deletions.
37 changes: 17 additions & 20 deletions robot-server/robot_server/protocols/protocol_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,49 +308,46 @@ def get_usage_info(self) -> List[ProtocolUsageInfo]:

return usage_info

# TODO (spp, 2024-07-22): get files referenced in runs as well
async def get_referenced_data_files(self, protocol_id: str) -> List[DataFile]:
"""Get a list of data files referenced in specified protocol's analyses and runs."""
"""Return a list of data files referenced in specified protocol's analyses and runs.
List returned is in the order in which the data files were uploaded to the server.
"""
# Get analyses and runs of protocol_id
select_referencing_analysis_ids = sqlalchemy.select(analysis_table.c.id).where(
analysis_table.c.protocol_id == protocol_id
)
select_referencing_run_ids = sqlalchemy.select(run_table.c.id).where(
run_table.c.protocol_id == protocol_id
)
# Get all entries in csv table that match the analyses
analysis_csv_file_ids = sqlalchemy.select(
# Get all entries in analysis_csv_table that match the analysis IDs above
select_analysis_csv_file_ids = sqlalchemy.select(
analysis_csv_rtp_table.c.file_id
).where(
analysis_csv_rtp_table.c.analysis_id.in_(select_referencing_analysis_ids)
)
run_csv_file_ids = sqlalchemy.select(run_csv_rtp_table.c.file_id).where(
# Get all entries in run_csv_table that match the run IDs above
select_run_csv_file_ids = sqlalchemy.select(run_csv_rtp_table.c.file_id).where(
run_csv_rtp_table.c.run_id.in_(select_referencing_run_ids)
)
# Get list of data file IDs from the entries
select_analysis_data_file_rows_statement = data_files_table.select().where(
data_files_table.c.id.in_(analysis_csv_file_ids)
)
select_run_data_file_rows_statement = data_files_table.select().where(
data_files_table.c.id.in_(run_csv_file_ids)
)

with self._sql_engine.begin() as transaction:
analysis_data_files_rows = transaction.execute(
select_analysis_data_file_rows_statement
).all()
run_data_files_rows = transaction.execute(
select_run_data_file_rows_statement
data_files_rows = transaction.execute(
data_files_table.select()
.where(
data_files_table.c.id.in_(select_analysis_csv_file_ids)
| data_files_table.c.id.in_(select_run_csv_file_ids)
)
.order_by(sqlite_rowid)
).all()

combine_data_file_rows = set(analysis_data_files_rows + run_data_files_rows)

return [
DataFile(
id=sql_row.id,
name=sql_row.name,
createdAt=sql_row.created_at,
)
for sql_row in combine_data_file_rows
for sql_row in data_files_rows
]

def get_referencing_run_ids(self, protocol_id: str) -> List[str]:
Expand Down
6 changes: 5 additions & 1 deletion robot-server/robot_server/runs/run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ async def create(
created_at=created_at,
protocol_id=protocol.protocol_id if protocol is not None else None,
)
run_time_parameters = self._run_orchestrator_store.get_run_time_parameters()
self._run_store.insert_csv_rtp(
run_id=run_id, run_time_parameters=run_time_parameters
)
await self._runs_publisher.start_publishing_for_run(
get_current_command=self.get_current_command,
get_recovery_target_command=self.get_recovery_target_command,
Expand All @@ -218,7 +222,7 @@ async def create(
run_resource=run_resource,
state_summary=state_summary,
current=True,
run_time_parameters=self._run_orchestrator_store.get_run_time_parameters(),
run_time_parameters=run_time_parameters,
)

def get(self, run_id: str) -> Union[Run, BadRun]:
Expand Down
8 changes: 6 additions & 2 deletions robot-server/robot_server/runs/run_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def get_all_csv_rtp(self) -> List[CSVParameterRunResource]:
with self._sql_engine.begin() as transaction:
csv_rtps = transaction.execute(select_all_csv_rtp).all()

return [_covert_row_to_csv_rtp(row) for row in csv_rtps]
return [_convert_row_to_csv_rtp(row) for row in csv_rtps]

def insert_csv_rtp(
self, run_id: str, run_time_parameters: List[RunTimeParameter]
Expand Down Expand Up @@ -543,9 +543,13 @@ def remove(self, run_id: str) -> None:
delete_commands = sqlalchemy.delete(run_command_table).where(
run_command_table.c.run_id == run_id
)
delete_csv_rtps = sqlalchemy.delete(run_csv_rtp_table).where(
run_csv_rtp_table.c.run_id == run_id
)
with self._sql_engine.begin() as transaction:
transaction.execute(delete_actions)
transaction.execute(delete_commands)
transaction.execute(delete_csv_rtps)
result = transaction.execute(delete_run)

if result.rowcount < 1:
Expand Down Expand Up @@ -574,7 +578,7 @@ def _clear_caches(self) -> None:
_run_columns = [run_table.c.id, run_table.c.protocol_id, run_table.c.created_at]


def _covert_row_to_csv_rtp(
def _convert_row_to_csv_rtp(
row: sqlalchemy.engine.Row,
) -> CSVParameterRunResource:
run_id = row.run_id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ stages:
- displayName: Liquid handling CSV file
variableName: liq_handling_csv_file
description: A CSV file that contains wells to use for pipetting
type: csv_file
file:
id: '{csv_file_id}'
name: 'sample_record.csv'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,250 @@
test_name: Test the /protocols/{protocolID}/dataFiles endpoint

marks:
- usefixtures:
- ot2_server_base_url

stages:
# The order of these data file uploads is important for this test,
# since the list of data files returned for the specified protocol is in upload order.
# The order in which the files are uploaded in this test is the same as the order in which
# these files are uploaded in the overall integration tests suite.
# Until we add data file cleanup after each test, maintaining this order within the suite
# will be important.

# sample_record -> test
# sample_plates -> sample_record
# test -> sample_plates
- name: Upload data file 1
request:
url: '{ot2_server_base_url}/dataFiles'
method: POST
files:
file: 'tests/integration/data_files/test.csv'
response:
save:
json:
data_file_1_id: data.id
data_file_1_name: data.name
status_code:
- 201
- 200

- name: Upload data file 2
request:
url: '{ot2_server_base_url}/dataFiles'
method: POST
files:
file: 'tests/integration/data_files/sample_record.csv'
response:
save:
json:
data_file_2_id: data.id
data_file_2_name: data.name
status_code:
- 201
- 200

- name: Upload data file 3
request:
url: '{ot2_server_base_url}/dataFiles'
method: POST
files:
file: 'tests/integration/data_files/sample_plates.csv'
response:
save:
json:
data_file_3_id: data.id
data_file_3_name: data.name
status_code:
- 201
- 200

- name: Upload protocol with CSV file ID
request:
url: '{ot2_server_base_url}/protocols'
method: POST
data:
runTimeParameterFiles: '{{"liq_handling_csv_file": "{data_file_1_id}"}}'
files:
files: 'tests/integration/protocols/basic_transfer_with_run_time_parameters.py'
response:
save:
json:
protocol_id: data.id
analysis_id: data.analysisSummaries[0].id
run_time_parameters_data1: data.analysisSummaries[0].runTimeParameters
strict:
json:off
status_code: 201
json:
data:
analysisSummaries:
- id: !anystr
status: pending
runTimeParameters:
- displayName: Liquid handling CSV file
variableName: liq_handling_csv_file
description: A CSV file that contains wells to use for pipetting
type: csv_file
file:
id: '{data_file_1_id}'
name: 'test.csv'

- name: Wait until analysis is completed
max_retries: 5
delay_after: 1
request:
url: '{ot2_server_base_url}/protocols/{protocol_id}'
response:
status_code: 200
json:
data:
analyses: []
analysisSummaries:
- id: '{analysis_id}'
status: completed
id: !anything
protocolType: !anything
files: !anything
createdAt: !anything
robotType: !anything
protocolKind: !anything
metadata: !anything
links: !anything

- name: Start a new analysis with a different CSV file
request:
url: '{ot2_server_base_url}/protocols/{protocol_id}/analyses'
method: POST
json:
data:
forceReAnalyze: true
runTimeParameterFiles:
liq_handling_csv_file: '{data_file_3_id}'
response:
strict:
- json:off
status_code: 201
json:
data:
- id: '{analysis_id}'
status: completed
- id: !anystr
status: pending
runTimeParameters:
- displayName: Liquid handling CSV file
variableName: liq_handling_csv_file
description: A CSV file that contains wells to use for pipetting
type: csv_file
file:
id: '{data_file_3_id}'
name: 'sample_plates.csv'

- name: Wait until analysis is completed
max_retries: 5
delay_after: 1
request:
url: '{ot2_server_base_url}/protocols/{protocol_id}'
response:
status_code: 200
json:
data:
analyses: []
analysisSummaries:
- id: '{analysis_id}'
status: completed
- id: !anystr
status: completed
id: !anything
protocolType: !anything
files: !anything
createdAt: !anything
robotType: !anything
protocolKind: !anything
metadata: !anything
links: !anything

- name: Create a run from the protocol and a CSV file
request:
url: '{ot2_server_base_url}/runs'
method: POST
json:
data:
protocolId: '{protocol_id}'
runTimeParameterFiles:
liq_handling_csv_file: '{data_file_1_id}'
response:
status_code: 201
save:
json:
run_id1: data.id
run_time_parameters_data2: data.runTimeParameters
strict:
json:off
json:
data:
id: !anystr
ok: True
createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))"
status: idle
runTimeParameters:
- displayName: Liquid handling CSV file
variableName: liq_handling_csv_file
description: A CSV file that contains wells to use for pipetting
type: csv_file
file:
id: '{data_file_1_id}'
name: 'test.csv'

- name: Create another run from the protocol and a different CSV file
request:
url: '{ot2_server_base_url}/runs'
method: POST
json:
data:
protocolId: '{protocol_id}'
runTimeParameterFiles:
liq_handling_csv_file: '{data_file_2_id}'
response:
status_code: 201
save:
json:
run_id2: data.id
run_time_parameters_data3: data.runTimeParameters
strict:
json:off
json:
data:
id: !anystr
ok: True
createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))"
status: idle
runTimeParameters:
- displayName: Liquid handling CSV file
variableName: liq_handling_csv_file
description: A CSV file that contains wells to use for pipetting
type: csv_file
file:
id: '{data_file_2_id}'
name: 'sample_record.csv'

- name: Fetch data files used with the protocol so far
request:
url: '{ot2_server_base_url}/protocols/{protocol_id}/dataFiles'
response:
status_code: 200
json:
meta:
cursor: 0
totalLength: 3
data:
- id: '{data_file_1_id}'
name: "test.csv"
createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))"
- id: '{data_file_2_id}'
name: "sample_record.csv"
createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))"
- id: '{data_file_3_id}'
name: "sample_plates.csv"
createdAt: !re_fullmatch "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+(Z|([+-]\\d{2}:\\d{2}))"
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ stages:
save:
json:
data_file_id: data.id
status_code: 201
status_code:
- 201
- 200
json:
data:
id: !anystr
Expand Down
Loading

0 comments on commit 90127ff

Please sign in to comment.