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

Check if CMO label already exists in SMILE #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ao508
Copy link
Collaborator

@ao508 ao508 commented Nov 22, 2024

Check if CMO label already exists in SMILE

Briefly describe changes proposed in this pull request:

1. Added a check to determine if a CMO sample label is already in use in SMILE for a different sample.

This may be the case when a request/sample is published to the IGO_NEW_REQUEST topic and may have a legacy CMO label in the data being published by/from IGO.

During an effort to recover legacy labels for samples we found that there are duplicates within the legacy IGO data.

The label-generator will now log and report such a case:

Incoming sample: 05469_AE_1 sample manifest contains an existing CMO label: C-N0LN75-P005-d that is already in use by another sample in SMILE. To prevent duplicate labels from getting persisted into SMILE, the new label generated will be used instead: C-N0LN75-P007-d03

We can catch these events in a datadog monitor.


Crossing T's and dotting I's

Please follow these checklists to help prevent any unexpected issues from being introduced by the changes in this pull request. If an item does not apply then indicate so by surrounding the line item with ~~ to strikethrough the text. See basic writing and formatting syntax for more information.

I. Data model checklist

Please follow these checks if any changes were made to any classes in the web, service, or persistence layers.

Code checks:

  • Unit tests were updated in relation to updates to the mocked test data.

II. Message handlers checklist:

  • Changes introduced affect the workflow of incoming messages.
  • Messages are following the expected workflow when published to the topic(s) changed or introduced in this pull request.
  • Unit tests were added or updated to ensure messages are handled as expected.

If no unit tests were updated or added, then please explain why: Tested on the dev server.

Please describe how the workflow and messaging was tested/simulated:

Describe your testing environment:

  • NATS [local, local docker, dev server, production]
  • Neo4j [local, local docker, dev server, production]
  • SMILE Server [local, local docker, dev server, production]
  • Message publishing simulation [nats cli, docker nats cli, smile publisher tool, other (describe below)]

Other: [insert details on how messages were published or simulated for testing]

II. Configuration and/or permissions checklist:

  • New topics were introduced.
  • The topics and appropriate permissions were updated in smile-configuration.
  • If applicable, a new account was set up and the account credentials and keys are checked into smile-configuration.
  • Account credentials and keys were shared with the appropriate parties.

General checklist:

  • All requested changes and comments have been resolved.
  • The commit log is comprehensible. It follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.

Added check to see if CMO label is laready in use in SMILE by another sample.

If label is already in use then the label generator assigns a fresh label to the current sample.

Signed-off-by: Angelica Ochoa <[email protected]>
@ao508 ao508 changed the title Init commit - check if label exists already in smile Check if CMO label already exists in SMILE Nov 22, 2024
@ao508 ao508 marked this pull request as ready for review November 22, 2024 17:42
@ao508 ao508 requested review from n1zea144 and qu8n November 22, 2024 18:08
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.

1 participant