update adapter version messages #10919
Conversation
dave-connors-3
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10919 +/- ##
=======================================
Coverage 89.19% 89.19%
=======================================
Files 183 183
Lines 23466 23470 +4
=======================================
+ Hits 20930 20934 +4
Misses 2536 2536
Flags with carried forward coverage won't be shown. Click here to find out more.
|
graciegoheen
added
the
ready_for_review
| @@ -130,7 +130,11 @@ def _get_plugin_msg_info( | |||
|
|
|||
| needs_update = False | |||
|
|
|||
| if plugin.major != core.major or plugin.minor != core.minor: | |||
| assert plugin.major is not None | |||
There was a problem hiding this comment.
asserts shouldn't be used in production, see: https://snyk.io/blog/the-dangers-of-assert-in-python/
| assert plugin.minor is not None | ||
| assert core.major is not None | ||
| assert core.minor is not None | ||
| if plugin.major != core.major or plugin.minor < core.minor: |
There was a problem hiding this comment.
So I talked with @colin-rogers-dbt + @mikealfare about this. I think we can get safely rid of this check entirely. Basically, we've decoupled the plugin (adapter specific) version and the core version. The majors no longer need to be equal, and the minor can be behind the core minor. The later might not actually be that uncommon.
All of this is the case because PyPI now handles this via dbt-adapters. Essentially, dbt-core specifies what dbt-adapters versions it is compatible with, and each dbt adapter also specifies what dbt-adapters versions it is compatible with. The result being, you'd have to intentionally force install incompatible versions for this check to be relevant. Thus, this check is unneeded.
There was a problem hiding this comment.
even better! you want to do the honors or should i?
Resolves #10230
Problem
The messages in
dbt --versiondon't reflect the fact that the minor version of an adapter could be ahead of the minor version of dbt coreSolution
update the semver check in
version.py!I added the assertions for mypy's sake, but can be convinced to do that a different way
Checklist