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

Remove all minimum/maximum and minLength/maxLength #45

Merged
merged 15 commits into from Jun 30, 2021

Conversation

dmosorast
Copy link

@dmosorast dmosorast commented Apr 29, 2021

Description of change

In a similar vein as #44, I would like to remove all of the restrictive properties from this schema so that the tap doesn't need to manually evolve in sync with the Chargebee API.

However, I don't have an account to test this (and I'm not familiar with the semantics of this data). So, I'd like to get someone familiar with the data and service, and ideally someone who can run the tap to confirm that it runs successfully with target-stitch if possible so that I can confirm that the Stitch Import API will accept actual records with the new schemas (to fix errors like in issue #43).

Manual QA steps

  • Requesting community assistance.

Risks

  • Unknown, needs investigated further.
  • Anecdotally, these should make the schemas less restrictive, so it should be backwards compatible, but may not be semantically desired, as mentioned above.

Rollback steps

  • revert this branch, release new patch version.

@dmosorast dmosorast added the help wanted Extra attention is needed label Apr 29, 2021
@dmosorast
Copy link
Author

In a wonderful lesson about the need to run the code for all PRs, no matter how small, this and #44 have invalid JSON due to trailing commas left behind.

I can't update this PR right now, but it definitely suffers from the same problem.

@dbshah1212 dbshah1212 removed the help wanted Extra attention is needed label Jun 9, 2021
@KAllan357 KAllan357 merged commit fcd8863 into master Jun 30, 2021
@KAllan357 KAllan357 deleted the remove-restrictive-json-schema-props branch June 30, 2021 14:58
@zachharris1 zachharris1 mentioned this pull request Jul 19, 2021
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.

None yet

5 participants