Release/0.1.0 - #1
fivetran-catfritz wants to merge 19 commits into
Conversation
fivetran-jamie
left a comment
There was a problem hiding this comment.
Looking pretty good! See comments -- also request to add a changelog
| {{ return([ | ||
| {"name": "_fivetran_deleted", "datatype": "boolean"}, | ||
| {"name": "_fivetran_synced", "datatype": dbt.type_timestamp()}, | ||
| {"name": "api_key_id", "datatype": dbt.type_string()}, | ||
| {"name": "character", "datatype": dbt.type_int()}, | ||
| {"name": "end_time", "datatype": dbt.type_int()}, | ||
| {"name": "input_token", "datatype": dbt.type_int()}, | ||
| {"name": "model", "datatype": dbt.type_string()}, | ||
| {"name": "num_model_request", "datatype": dbt.type_int()}, | ||
| {"name": "object", "datatype": dbt.type_string()}, | ||
| {"name": "project_id", "datatype": dbt.type_string()}, | ||
| {"name": "start_time", "datatype": dbt.type_int()}, | ||
| {"name": "user_id", "datatype": dbt.type_string()}, | ||
| ]) }} |
There was a problem hiding this comment.
This format works, but it is a little different from our other packages where we set the column dictionary as columns and return that
I see we do it the old/typical way for tables with custom columns here -- should we stay consistent?
| @@ -0,0 +1,16 @@ | |||
| {% macro get_audio_speech_columns() %} | |||
| {{ return([ | |||
| {"name": "_fivetran_deleted", "datatype": "boolean"}, | |||
There was a problem hiding this comment.
| {"name": "_fivetran_deleted", "datatype": "boolean"}, | |
| {"name": "_fivetran_deleted", "datatype": dbt.type_timestamp()}, |
Request to apply this across the board
There was a problem hiding this comment.
Did you mean dbt.type_boolean()?
| @@ -0,0 +1,593 @@ | |||
| _fivetran_id,project_id,line_item,amount_value,amount_currency,start_time,_fivetran_deleted,_fivetran_synced | |||
There was a problem hiding this comment.
Claude suggests adding a line here with a null project_id so that the "allocated" fallback method is tested
| ) }} | ||
|
|
||
| with prod as ( | ||
| select * |
There was a problem hiding this comment.
Request to use dbt_utils.star (in other consistency tests as well)
| estimated_cost_usd_amount: "{{ 'float64' if target.type == 'bigquery' else 'float' }}" | ||
| compliance_users: | ||
| +column_types: | ||
| created_at: timestamp |
There was a problem hiding this comment.
Request to set the data type of all columns so that the DAG site is accurate
| openai__completion_passthrough_metrics: | ||
| - name: "field_id" | ||
| alias: "field_name" | ||
| transform_sql: "cast(field_id as int64)" |
There was a problem hiding this comment.
transform_sql should reference the alias
|
|
||
| ### Enable/Disable models | ||
|
|
||
| > _This step is optional if you are unioning multiple connections together in the previous step. The `union_data` macro will create empty staging models for sources that are not found in any of your OpenAI schemas/databases. However, you can still leverage the below variables if you would like to avoid this behavior._ |
There was a problem hiding this comment.
| > _This step is optional if you are unioning multiple connections together in the previous step. The `union_data` macro will create empty staging models for sources that are not found in any of your OpenAI schemas/databases. However, you can still leverage the below variables if you would like to avoid this behavior._ | |
| > _This step is optional if you are unioning multiple connections together in the previous step. The `union_connections` macro will create empty staging models for sources that are not found in any of your OpenAI schemas/databases. However, you can still leverage the below variables if you would like to avoid this behavior._ |
|
|
||
| > _This step is optional if you are unioning multiple connections together in the previous step. The `union_data` macro will create empty staging models for sources that are not found in any of your OpenAI schemas/databases. However, you can still leverage the below variables if you would like to avoid this behavior._ | ||
|
|
||
| This package takes into consideration that not every OpenAI Platform/Enterprise account syncs every source table, and allows you to disable the corresponding functionality for any of them: `cost`, `completion`, `embedding`, `audio_transcription`, `audio_speech`, `image`, `moderation`, `web_search_call`, `file_search_call`, `codex_usage`, `codex_usage_model`, `project`, `project_api_key`, `project_user`, `project_user_role`, `project_role`, `users_role`, `groups`, and `invite`. `users` isn't included here — it's the spine of `openai__user_summary` with no partial-value alternative, so `stg_openai__users` and `openai__user_summary` always build. |
There was a problem hiding this comment.
The wording about users is a lil clunky -- I'd maybe say "all tables except users"
| "openai__user_summary" | ||
| ] | ||
|
|
||
| supported_vars: |
There was a problem hiding this comment.
is openai_model_family_overrides meant to be excluded from quickstart?
| coalesce(cast(project_id as {{ dbt.type_string() }}), '__none__') as project_id, | ||
| coalesce(cast(user_id as {{ dbt.type_string() }}), '__none__') as user_id, | ||
| {{ "'__none__'" if product_config.no_model is defined else "coalesce(cast(model as " ~ dbt.type_string() ~ "), '__none__')" }} as model, |
There was a problem hiding this comment.
Same question -- should we leave these as null?
Initial release v0.1.0.