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

feat: set annotations in an async way when submitting a manifest -- new changes added #1452

Merged
merged 23 commits into from
Jul 19, 2024

Conversation

linglp
Copy link
Contributor

@linglp linglp commented Jun 27, 2024

Context

See original PR for context and benchmarking result: #1440

Changes:

Related to: https://sagebionetworks.jira.com/browse/FDS-2179
Related to: https://sagebionetworks.jira.com/browse/FDS-2181

  • created the decorator that works for async format_row_annotation to accommodate the current expected behavior: when trying to add annotations to an entity id, and that entity id is in the trash can, return a warning instead of an error.
  • added unit tests for the following:
  1. added a test for format_row_annotation to make sure that when entity id is in trash can, the function would return None and a warning will be logged. This is the current behavior. If not followed, integration test: test_upsertTable would fail.
  2. added a test for process_store_annos to make sure that storing annotations is not triggered when annotations are empty
  3. update the code to accommodate different variants of "EntityId" and "Id"
  4. Added tests to ensure that variations of "EntityId" and "Id" get handled.

Note: format_row_annotations is still under tested because of technical debt in previous development of schematic (independent of async). Please see FDS 2138 for further testing and refactoring this function

Tests

I re-ran all the tests, including all the integration tests to make sure that they are all working.

@linglp linglp marked this pull request as ready for review June 28, 2024 17:49
Copy link

@linglp
Copy link
Contributor Author

linglp commented Jul 17, 2024

@andrewelamb @GiaJordan I ran all tests again after merging with the latest develop, and they all passed.

@linglp linglp merged commit 23d77c8 into develop Jul 19, 2024
4 checks passed
@linglp linglp deleted the develop-async-set-annotation branch July 19, 2024 21:32
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.

2 participants