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.

Sign up for GitHub

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

Jump to bottom

framework: unit test JsonSchemaResolver #2722

Closed
sherifnada opened this issue Apr 2, 2021 · 0 comments · Fixed by #3223
Closed

framework: unit test JsonSchemaResolver #2722

sherifnada opened this issue Apr 2, 2021 · 0 comments · Fixed by #3223
Assignees
@sherifnada
Labels
CDK Connector Development Kit type/enhancement New feature or request
Milestone
Core - 2021-05-07

Comments

@sherifnada
Copy link
Contributor

No description provided.

@sherifnada sherifnada added the type/enhancement New feature or request label Apr 2, 2021
@sherifnada sherifnada added this to the Core - 2021-04-09 milestone Apr 5, 2021
@cgardens cgardens added the CDK Connector Development Kit label Apr 5, 2021
@sherifnada sherifnada removed this from the Core - 2021-04-09 milestone Apr 12, 2021
@sherifnada sherifnada mentioned this issue Apr 30, 2021
Closed
@sherifnada sherifnada added this to the Core - 2021-05-07 milestone May 3, 2021
@sherifnada sherifnada mentioned this issue May 4, 2021
Merged
davinchia pushed a commit that referenced this issue May 5, 2021
@sherifnada
Test JSONSchema resolver (#3223)
c335668 
closes #2722

I don't love the current structure. There is a number of improvements that can be made:

JsonSchemaResolver can be refactored to have the following signature:

def __init__(schemas: Mapping[str, any], refs: Mapping[str, any])
  # schemas are all json files in `schemas/`, refs are all json files in `schemas/shared/`
Then we can thoroughly test the resolving behavior in JsonSchemaResolver independently from the logic for where we actually load schemas from.

I was tempted to make this refactor now but I'd rather cover more ground with tests then revisit. I created an issue to track this: #3222
@sherifnada sherifnada self-assigned this May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sherifnada sherifnada

Labels
CDK Connector Development Kit type/enhancement New feature or request
Projects
None yet
Milestone
Core - 2021-05-07
Development

Successfully merging a pull request may close this issue.

Test JSONSchema resolver airbytehq/airbyte
2 participants
@sherifnada @cgardens