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

[Bug] dbt_project.yml does not fully support semantic versioning #4453

Closed
1 task done
b0lle opened this issue Dec 8, 2021 · 3 comments · Fixed by #4644
Closed
1 task done

[Bug] dbt_project.yml does not fully support semantic versioning #4453

b0lle opened this issue Dec 8, 2021 · 3 comments · Fixed by #4644
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors!

Comments

@b0lle
Copy link

b0lle commented Dec 8, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

One can read in the docs that semantic versioning is supported. If so, pre-versions like 1.0.0-pre.1 should be supportes (as the link in the docs says).
Currently the version number causes a Runtime Error:

Encountered an error while reading the project:
  ERROR: Runtime Error
  at path ['version']: '0.0.0-pre.1' is not valid under any of the given schemas
# dbt_project.yml

version: '1.0.0-pre.1'

Expected Behavior

Version 1.0.0-pre.1 should be an accepted version, because it follows the rules of semantic versioning

Steps To Reproduce

No response

Relevant log output

No response

Environment

- OS: macOS 11.5.2
- Python: 3.9.7
- dbt: 21.1

What database are you using dbt with?

No response

Additional Context

No response

@b0lle b0lle added bug Something isn't working triage labels Dec 8, 2021
@jtcohen6
Copy link
Contributor

@b0lle Thanks for opening!

The codepath for this one is fairly straightforward, and leads us to a comment indicating that version only supports "semver lite":

version: Union[SemverString, float]

class SemverString(str, SerializableType):
def _serialize(self) -> str:
return self
@classmethod
def _deserialize(cls, value: str) -> 'SemverString':
return SemverString(value)
# this does not support the full semver (does not allow a trailing -fooXYZ) and
# is not restrictive enough for full semver, (allows '1.0'). But it's like
# 'semver lite'.
register_pattern(
SemverString,
r'^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(\.(?:0|[1-9]\d*))?$',
)

At the same time, the docs say:

The version must be in a semantic version format, e.g. 1.0.0

Why not support real semver? Well... the version field isn't used for anything today. It serves no functional purpose. But it is required, in all projects—and I could imagine it being useful for something, someday. In particular, there's no way currently to see the currently installed versions of any packages, because there's no actual correspondence between the version in their dbt_project.yml, and the version (GitHub tag) specified in packages.yml. But imagine if they did correspond—if we did a good job of enforcing their alignment—which would unlock the ability for a dbt deps freeze to do something like:

$ dbt deps freeze
dbt-utils==0.7.1
snowplow==0.13.0

Updates available for packages: ['dbt-labs/dbt_utils', 'dbt-labs/snowplow']
Update your versions in packages.yml, then run dbt deps

Currently, that version report is only available by running dbt deps, i.e. actually re-installing packages based on the specification in packages.yml.

So, I think we have two valid options:

  1. Validate this via an actual semantic version specification, such as by using dbt.semver.VersionSpecifier.from_version_string
  2. Support any string in the version field, because it serves no functional purpose today. (If we do this now, though, it will be much harder to do anything useful with it in the future.)

I'm inclined to prefer the former: This should be a semantic version, rather than an anything-goes string. We already have logic (a whole module, even) dedicated to validating semantic versions.

I think the lift here would be pretty small, so I'm going to mark this a good first issue! Would you be interested in contributing the fix? :)

@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! and removed triage labels Dec 15, 2021
@alswang18
Copy link
Contributor

I can handle this if no one else is @jtcohen6.

@b0lle
Copy link
Author

b0lle commented Jan 31, 2022

I'm sorry @jtcohen6, I just didn't see your post.
Thanks for the explanation. Even though dbt does not use this flag, we have a use case where we extract the version from dbt_project to feed our CI/CD pipeline that creates a package in the package registry.

Thanks @alswang18 for creating the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants