-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: support for custom grain #55
Conversation
This commit adds a new `BaseModel.DEPRECATED` key that can be added to the dataclasses `metadata` dict to indicate that this field is deprecated.
By the way: I had another PR for this that kept the old |
This commit introduces a new `DeprecatedMixin` class that can be added to any model class to mark that class as deprecated. It will throw a warning if the user tries to instantiate the class.
b98ab02
to
564c332
Compare
@@ -8,7 +8,9 @@ kindFormat: '### {{.Kind}}' | |||
changeFormat: '* {{.Body}}' | |||
kinds: | |||
- label: Breaking Changes | |||
auto: major |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're under 1.0
we can make breaking changes in minor versions so I changed it.
564c332
to
d34961f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handling for custom grain looks right, but can we add some tests? Not sure if you have e2e tests for this stuff but it would be great to be able to confirm you can send a request with custom grain and get back an expected result
@courtneyholcomb about testing:
|
Since we introduced custom grains, the old `queryable_granularities` is deprecated in favor of the new `queryable_time_granilarities`, which is just a simple list of strings. This commit marks the `TimeGranularity` class and all granularity fields that return it as deprecated.
This commit changes the `OrderByGroupBy` class to use `str` instead of the deprecated `TimeGranularity` enum as its input grain. We can do this without a deprecation because we haven't released the SDK since the order by refactor, so we can just change it.
We need to call `BaseModel._register_subclasses` otherwise models will fail to use `camelCase` and raise deprecation warnings. That is done in `dbtsl.models.__init__`. If the user never explicitly imports that, this won't get called, and they might get an error. This fixes that by adding an explicit call to it on the library init.
We are raising deprecation warnings from the GQL client when we instantiate the models. To avoid the warning spam, we filter those warnings out. They should only be display if the user uses any deprecated class, not us.
ccd5cde
to
ed78b15
Compare
Summary
This PR adds support for custom grain and deprecates old fields that use the
TimeGranularity
enum. I also deprecatedTimeGranularity
itself.I recommend reviewing commit by commit :)