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

Migrated jsonschema.RefResolver to referencing.Registry #160

Merged
merged 6 commits into from
Sep 28, 2023
Merged

Migrated jsonschema.RefResolver to referencing.Registry #160

merged 6 commits into from
Sep 28, 2023

Conversation

codecae
Copy link
Contributor

@codecae codecae commented Sep 28, 2023 •
edited
Loading

Resolves #135

Description

Migrate from jsonschema.RefResolver to referencing.Registry in support of jsonschema 4 upgrade

Checklist

@cla-bot
Copy link

cla-bot bot commented Sep 28, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @codecae

@codecae codecae mentioned this pull request Sep 28, 2023
Closed
3 tasks
@codecae codecae force-pushed the feature/referencing branch from cf6470f to c7763d7 Compare September 28, 2023 03:50
@cla-bot
Copy link

cla-bot bot commented Sep 28, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @codecae

@codecae codecae force-pushed the feature/referencing branch from c7763d7 to 1eb3294 Compare September 28, 2023 03:52
@cla-bot
Copy link

cla-bot bot commented Sep 28, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @codecae

@codecae codecae force-pushed the feature/referencing branch from 1eb3294 to 83063a6 Compare September 28, 2023 03:53
@cla-bot
Copy link

cla-bot bot commented Sep 28, 2023

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @codecae

@codecae codecae changed the title feat: migrated jsonschema.RefResolver to referencing.Registry Migrated jsonschema.RefResolver to referencing.Registry Sep 28, 2023
@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023 

Successfully installed anyio-4.0.0 black-23.9.1 certifi-2023.7.22 cfgv-3.4.0 distlib-0.3.7 execnet-2.0.2 filelock-3.12.4 h11-0.14.0 httpcore-0.18.0 httpx-0.25.0 identify-2.5.29 idna-3.4 iniconfig-2.0.0 isort-5.12.0 mypy-1.5.1 mypy-extensions-1.0.0 nodeenv-1.8.0 packaging-23.1 pathspec-0.11.2 platformdirs-3.10.0 pluggy-1.3.0 pre-commit-3.4.0 pytest-7.4.2 pytest-xdist-3.3.1 ruff-0.0.260 sniffio-1.3.0 types-Jinja2-2.11.9 types-MarkupSafe-1.1.10 types-PyYAML-6.0.12.12 types-jsonschema-4.19.0.3 types-python-dateutil-2.8.19.14 virtualenv-20.24.5
Finished syncing dependencies
cmd [1] | pytest -n auto tests
================================================================================ test session starts ================================================================================
platform linux -- Python 3.11.4, pytest-7.4.2, pluggy-1.3.0
rootdir: /home/codec/git/dbt-semantic-interfaces
plugins: anyio-4.0.0, xdist-3.3.1
8 workers [133 items]   
.....................................................................s..........................................s....................                                         [100%]
========================================================================== 131 passed, 2 skipped in 2.41s ===========================================================================

@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023

I have signed the CLA but the workflow seems to be stuck. Anything I need to do on my end?

@QMalcolm
Copy link
Collaborator

I have signed the CLA but the workflow seems to be stuck. Anything I need to do on my end?

I was looking at our list of people who've signed the CLA, and I do see you in there as @codecae. For whatever reason if the submitted GitHub Account Name has an @ at the start, it gets dropped. I'm going to see if I'm allowed to manually edit our list of github account names that have signed the CLA. You may need to sign the CLA again though and use codecae for the GitHub Account Name field instead of @codecae

@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023 •
edited
Loading

I wondered if that might be the issue. I figured it was looking for what the CLA bot posted in the automated comment

@cla-bot cla-bot bot added the cla:yes label Sep 28, 2023
@QMalcolm
Copy link
Collaborator

@codecae I'll see about getting the CLA bot message to be more clear! It probably puts the @ in the message to notify the specific users, but that seems confusing without some level of clarification 🙃 Additionally, apparently "bad things happen"™️ when we manually edit the list of people who've signed the CLA, so resigning it is probably the best way forward

@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023

I just signed it all again and it seemed to work just fine 👍

@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023

Should be squeaky clean now... linter seems happy.

@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023

Would you prefer a squash to lower the noise floor a bit on the commit history?

@QMalcolm
Copy link
Collaborator

Would you prefer a squash to lower the noise floor a bit on the commit history?

Yea we'll probably end up squash merging, just to keep the history clear of cleanup commits

@codecae
Copy link
Contributor Author

codecae commented Sep 28, 2023

I ran the linter in containers for 3.8-3.11 I think everything should be good now. Sorry for the small misses here.

@QMalcolm
Copy link
Collaborator

I ran the linter in containers for 3.8-3.11 I think everything should be good now. Sorry for the small misses here.

No worries! The number of times the linter has caught me is too many to count. Thank you for sticking with it, this work is incredibly helpful 🙂

QMalcolm
QMalcolm approved these changes Sep 28, 2023
View reviewed changes
Copy link
Collaborator

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you again for doing this work! This looks great 🚀

@QMalcolm QMalcolm merged commit d3c26df into dbt-labs:main Sep 28, 2023
7 checks passed
@zli06160
Copy link

Super quick reaction! Thank you 🍾

@der-eismann der-eismann mentioned this pull request Sep 29, 2023
Closed
4 tasks
@codecae codecae deleted the feature/referencing branch September 29, 2023 11:29
@codecae
Copy link
Contributor Author

codecae commented Sep 29, 2023

Thank you again for doing this work! This looks great 🚀

Glad to help!

@codecae codecae mentioned this pull request Oct 4, 2023
Closed
3 tasks
QMalcolm pushed a commit that referenced this pull request Oct 4, 2023
@codecae @QMalcolm
Migrate from JsonSchema 3 to JsonSchema 4 (#160)
82b5bb6 
* feat: migrated jsonschema.RefResolver to referencing.Registry

* fixed typo in changie output for PR 160

* corrected whitespace for pre-commit linter

* corrected linter violations

* removed subscript from Resource

* clean up linter violation for Tuple
QMalcolm added a commit that referenced this pull request Oct 5, 2023
@QMalcolm
Merge pull request #165 from dbt-labs/0.2.latest--backport-jsonschema-4
fdbe1c0 
Migrate from JsonSchema 3 to JsonSchema 4 (#160)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QMalcolm QMalcolm QMalcolm approved these changes

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] Allow jsonschema 4 as dependency
3 participants
@codecae @QMalcolm @zli06160