[Feature] Enable User models by default #17
Conversation
There was a problem hiding this comment.
@fivetran-avinash thanks for this update! A few change requests before this will be ready for approval.
CHANGELOG.md
Outdated
|
|
||
| ## Breaking Changes | ||
| - Enabled `servicenow__user_aggregated` and `servicenow__user_enhanced` by default by changing the default `servicenow__using_roles` value to true. | ||
| - By setting the `servicenow__using_roles` variable to true, we now also enable the upstream `stg_servicenow_*` models that flow into these user tables, which derive from the `sys_user_grmember`, `sys_user_has_role`, and `sys_user_role` source tables. |
There was a problem hiding this comment.
I want to clearly callout what new models will be materialized in the warehouse by default. Can you add onto this entry sub-bullets which list out all the new models which will be enabled by default.
Additionally, we should mention that Quickstart will dynamically handle the enable/disabling based on the existence of the source tables or via the Quickstart data model selection tab. However, non Quickstart users will be required to define the variable themselves in their dbt_project.yml if they do not have the required source tables.
CHANGELOG.md
Outdated
| ## Breaking Changes | ||
| - Enabled `servicenow__user_aggregated` and `servicenow__user_enhanced` by default by changing the default `servicenow__using_roles` value to true. | ||
| - By setting the `servicenow__using_roles` variable to true, we now also enable the upstream `stg_servicenow_*` models that flow into these user tables, which derive from the `sys_user_grmember`, `sys_user_has_role`, and `sys_user_role` source tables. | ||
| - Because this change will introduce new end model tables to users because they were initially disabled by default, we've classified this as a breaking change. |
There was a problem hiding this comment.
This is also a breaking change because we are changing the default behavior of a variable. Please also include here that users who do not have these source tables will need to define the variable in their dbt_project.yml
There was a problem hiding this comment.
@fivetran-avinash approved with just a few final requests before release review. Thanks!
CHANGELOG.md
Outdated
| - Staging models now also enabled by default include: | ||
| - `stg_servicenow__sys_user_grmember` | ||
| - `stg_servicenow__sys_user_has_role` | ||
| - `stg_servicenow__sys_user_role` |
There was a problem hiding this comment.
Can you also include the base tables.
CHANGELOG.md
Outdated
| - `stg_servicenow__sys_user_has_role` | ||
| - `stg_servicenow__sys_user_role` | ||
| - Because this change will introduce new end model tables to users because they were initially disabled by default and changes the default behavior of a variable, we've classified this as a breaking change. | ||
| - Quickstart users will be able to handle disabling of these tables by [utilizing the Quickstart data model tab](https://fivetran.com/docs/using-fivetran/fivetran-dashboard/transformations#datamodels) within your ServiceNow connection, and choose to un-sync the tables required for these models. |
There was a problem hiding this comment.
| - Quickstart users will be able to handle disabling of these tables by [utilizing the Quickstart data model tab](https://fivetran.com/docs/using-fivetran/fivetran-dashboard/transformations#datamodels) within your ServiceNow connection, and choose to un-sync the tables required for these models. | |
| - Quickstart users will be able to handle disabling of these tables by [utilizing the Quickstart data model tab](https://fivetran.com/docs/transformations/quickstart) within your ServiceNow connection, and choose to un-sync the tables required for these models. |
Let's use this link instead.
CHANGELOG.md
Outdated
| - `stg_servicenow__sys_user_role` | ||
| - Because this change will introduce new end model tables to users because they were initially disabled by default and changes the default behavior of a variable, we've classified this as a breaking change. | ||
| - Quickstart users will be able to handle disabling of these tables by [utilizing the Quickstart data model tab](https://fivetran.com/docs/using-fivetran/fivetran-dashboard/transformations#datamodels) within your ServiceNow connection, and choose to un-sync the tables required for these models. | ||
| - Non-Quickstart users who do not have these source tables will need to re-define the variable in their `dbt_project.yml`. [See the README](https://github.com/fivetran/dbt_servicenow/blob/main/README.md#optional-step-4-additional-configurations) for more instructions. |
There was a problem hiding this comment.
| - Non-Quickstart users who do not have these source tables will need to re-define the variable in their `dbt_project.yml`. [See the README](https://github.com/fivetran/dbt_servicenow/blob/main/README.md#optional-step-4-additional-configurations) for more instructions. | |
| - Non-Quickstart users who do not have these source tables will need to define the variable in their `dbt_project.yml`. [See the README](https://github.com/fivetran/dbt_servicenow/blob/main/README.md#optional-step-4-additional-configurations) for more instructions. |
It's more likely this variable wasn't defined at all in the past. So we should clarify that this needs to be defined as opposed to re-define.
PR Overview
This PR will address the following Issue/Feature: [#14] and [#16]
This PR will result in the following new package version: 0.4.0
New models are now enabled by default so this will create new end models for customers, particularly in Quickstart.
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
Breaking Changes
servicenow__user_aggregatedandservicenow__user_enhancedby default by changing the defaultservicenow__using_rolesvalue to true.servicenow__using_rolesvariable to true, we now also enable the upstreamstg_servicenow_*models that flow into these user tables, which derive from thesys_user_grmember,sys_user_has_role, andsys_user_rolesource tables.Documentation Update
servicenow__using_rolesis now set to true by default.Under the Hood
servicenow__using_rolesis set to False (since it's now True by default on all models).PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please share any and all of your validation steps:
Ran and validated that the tables compiled when
servicenow__using_roleswas True or False, then ensured validation tests passed on our development data.If you had to summarize this PR in an emoji, which would it be?
🍾