Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DPL-753-2 process new QC fields from TOL #40

Merged
merged 6 commits into from
Jul 17, 2023

Conversation

Sangeetha-Bheeman
Copy link
Contributor

@Sangeetha-Bheeman Sangeetha-Bheeman commented Jun 29, 2023

Closes #

Changes proposed in this pull request:

  • In tol_lab_share project, add new message properties for each of the new qc fields and add them as properties for a sample with no validation apart from checking is a string (check how it is with CommonName) (in message_properties/sample.py)

  • Create new message for qc_results traction to include the new fields in the payload we send

  • Add the new properties into the qc results traction message that we will send (in message_properties/labware.py). We need also to add the attributes to qc results traction message interface to add the new fields

  • Modify the CreateLabwareProcessor to send the message to qc_results traction endpoint if the creation of samples was successful in previous step

  • In case of errror while sending qc_result request, append the errors to the feedback message

  • Add the required tests.

  • ...

@Sangeetha-Bheeman Sangeetha-Bheeman marked this pull request as draft June 29, 2023 11:16
@Sangeetha-Bheeman Sangeetha-Bheeman linked an issue Jun 29, 2023 that may be closed by this pull request
6 tasks
@Sangeetha-Bheeman Sangeetha-Bheeman marked this pull request as ready for review July 14, 2023 12:53
Copy link
Contributor

@emrojo emrojo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments, but all ok


EBI_TAXONOMY_URL = "https://www.ebi.ac.uk/ena/taxonomy/rest/tax-id"

# Options: 'json' or 'binary'
SELECTED_ENCODER_FOR_FEEDBACK_MESSAGE = "binary"

# Validate SSL certificate chain when connecting to https
CERTIFICATES_VALIDATION_ENABLED = True
CERTIFICATES_VALIDATION_ENABLED = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was for testing locally, we can roll it back to True to test it in github actions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +229 to +240
def requests(self, position: int) -> TractionQcMessageRequestInterface:
"""Returns the request at position position from this message. If there is no requesrt there it
will create a new instance and return it.
Parameters:
position (int) position that we want to return
Returns:
OutputTractionMessageRequest with the request required
"""
if position not in self._requests:
self._requests[position] = TractionQcMessageRequest()

return self._requests[position]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this requests from the new qc result message sent to traction?

@Sangeetha-Bheeman Sangeetha-Bheeman merged commit 0a2ddf3 into develop Jul 17, 2023
4 checks passed
@Sangeetha-Bheeman
Copy link
Contributor Author

From Steve - Update from meeting 27/07: date_required_by and reason_for_priority are no longer required and can be removed from this schema. priority_level is not a qc_result attribute but should be moved to sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DPL-753-2 Process the message in consumer for new schema with QC fields
2 participants