Handle patch error messages with SDK #193
Conversation
beartype kept complaining that node project is not of type BaseNode, so I removed the typing for now for easy testing and will add it after and debug it
* can create a project node * can get a project node * cannot deserialize a project node from API to a project node * cannot assert that they are equal for now
* can create a project node * can get a project node * cannot deserialize a project node from API to a project node * cannot assert that they are equal for now
* can create a project node * can get a project node * cannot deserialize a project node from API to a project node * cannot assert that they are equal for now
* can create a project node * can get a project node * cannot deserialize a project node from API to a project node * cannot assert that they are equal for now
using a helper function for integration test functions because there is a lot of common code between all integration tests
using a helper function for integration test functions because there is a lot of common code between all integration tests
inventory integration test is failing with an API error of: `Bad uuid: 27da914c-65f1-4e8f-9797-5633d2fe0608 provided`
* test_integration_simple_process
* runs but cannot deserialize
* test_integration_complex_process
* takes forever to run and the schema validation comes out wrong
wrote the first draft, but getting `CRIPTOrphanedMaterialError`
citation currently missing from the API response of the project
getting orphaned nodes
* starting on update integration tests * changed the line separators to easier read through the code * changed the line separators to easier read through the code * updated test_integration.py to be easier to read in terminal * update integration test for test_collection.py * update integration test for test_computation.py * wrote update integration test for test_computational_process.py * wrote update integration test for test_data.py * wrote update integration test for test_experiment.py * wrote update integration test for test_inventory.py * wrote update integration test for test_material.py * updated the update integration test for test_computation.py * updated the update integration test for test_computational_process.py * updated the update integration test for test_data.py * updated the update integration test for test_experiment.py * updated the update integration test for test_collection.py * updated the update integration test for test_inventory.py * updated the update integration test for test_material.py * updated the update integration test for test_material.py * wrote the update integration test for test_project.py * wrote the update integration test for test_process.py * made reference fixture into named arguments * wrote the update integration test for test_reference.py * wrote the update integration test for test_citation.py * wrote the update integration test for test_algorithm.py * updated the update integration test for test_collection.py * updated the update integration test for test_collection.py * updated the update integration test for test_computational_forcefiled.py * wrote the update integration test for test_condition.py * wrote the update integration test for test_equipment.py * updated integration update test * wrote the update integration test for test_ingredient.py * cleaned up test_ingredient.py integration test * wrote the update integration test for test_ingredient.py * wrote the update integration test for test_parameter.py * wrote the update integration test for test_property.py * wrote and update quantity integration test cleaned up integration test for quantity so it is easier to read wrote integration test for quantity so the value is changed for the integration test to a unique number * wrote the update integration test for test_software.py * wrote the update integration test for test_software_configuration.py * wrote the update integration test for test_file.py * updated update integration test for test_reference.py * update test_software.py * update test_parameter.py * updated formatting * formatted with black * commented out unused import * mid debugging, but fixed a bug already * updated test_software_configuration.py update integration test * updated docstrings for test_integration.py * formatted with black * formatted with trunk's isort * identified the root of an issue, that I can't fix right now * condense citation.reference and make debugging easier. * undo the reference thing * prevent empty saves * minor problem * remove dependency of paginator for save to work. Use request.get directly * fix search lower to snake_case * paginator fixfix * half way there --------- Co-authored-by: Ludwig Schneider <ludwigschneider@uchicago.edu>
|
Can we merge this? I know that 3 tests aren’t passing. But I don’t think they will in the near future. |
|
The user testing should definitely happen with this branch merged. The save function is better and different to the current develop |
I'm thinking what if the way we are handling update here is wrong and we have to change our whole approach for it, so until we know that it is working I'm thinking we shouldn't merge it yet also we are not testing the SDK very heavily either because as you said if we try to update an inventory the SDK will not update it in the API, so all of those nodes would likely become invalid, our tests for update are very limited currently |
|
Let's merge when the tests pass |
|
the tests that we have for update are good, but they are very small, in the future we have to be sure to add heftier tests that tests changing a multiple trees within nodes and be sure it passes that test as well |
| @@ -110,13 +110,21 @@ class CRIPTAPISaveError(CRIPTException): | |||
| http_code: str | |||
| api_response: str | |||
|
|
|||
| def __init__(self, api_host_domain: str, http_code: str, api_response: str): | |||
| def __init__(self, api_host_domain: str, http_code: str, api_response: str, patch_request: bool, pre_saved_nodes: Optional[Set[str]] = None, json_data: Optional[str] = None): | |||
There was a problem hiding this comment.
this exception class will need clean up later
There was a problem hiding this comment.
I recommend passing in the http method as a string or something, we would then manipulate the string to be uppercase so the errors are always uniform
having a bool for it works right now because we only have POST and PATCH, but in the future when we start to have DELETE and PUT this will not be flexible
I think it is fine for now to get the project done, but will need refactor in the future to be more maintainable and more friendly
Description
We read the error message from the back end in patches.
And we analyze them to adjust our JSON to be compliant.
Similar strategy as with
Bad UUID.Changes
Minor changes to the save routine to catch and handle those errors.
The serialization now optionally allows the suppression attributes, to be compliant with expectation of back end.
This also refactors the internal save function a bit, making it clearer, of what needs to be omitted from JSON for the actual save.
Tests
None, yet.
We need to come up with some.
Known Issues
Only implemented for fields on the
/path.I haven't seen another response, so if we have an example for deeper modification, we can/have to implement it.
Notes
Checklist
CONTRIBUTORS.md) in the pull request source branch.