1. bulk_mode. 2. _execute_step. 3. Type checking. #1632
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. I forgot bulk_mode in the schema because there were no example mapping files with it. Fixed now. 2. Renamed _load_mapping to _execute_step because _load_mapping always implied to me that it was loading the mapping file. 3. I added stronger type checking which is how I found problem 1. Integration tests don't use bulk_mode but unit tests do.
davisagli
reviewed
Mar 23, 2020
| def _load_mapping(self, mapping): | ||
| def _execute_step( | ||
| self, mapping: MappingStep | ||
| ) -> Union[DataOperationJobResult, MagicMock]: |
There was a problem hiding this comment.
I don't like having to include mocks in the type signature. Is there no way to tell pydantic to always allow them?
There was a problem hiding this comment.
I think you mean typeguard.
And yes, there is a plan to fix it at the Typeguard level but not merged yet.
I'll bug the maintainer again, but if he doesn't respond we might have to figure out whether to fork, abandon or continue to work around small bugs in typeguard. I found another one on the weekend. Easy to fix, but only if he accepts my PRs.
|
@prescod the tests are failing because this imports from typing_extensions which is not in requirements |
davisagli
approved these changes
Mar 24, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I forgot bulk_mode in the schema because there were no example mapping files with it. Fixed now.
Renamed _load_mapping to _execute_step because _load_mapping always implied to me that it was loading the mapping file. This is subjective so tell me if you disagree.
I added stronger type checking which is how I found problem 1. Integration tests don't use bulk_mode but unit tests do
I deleted the test that checks what happens if a bulk_mode is set to a wrong value in mapping because the mapping parser should handle that now and it is hard to trigger with both pytest and typeguard working at the same time. In any case, the Salesforce API would give you an error pretty quickly if you found a way around all of this type checking...