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

Release/v0.17.0 #169

Merged
merged 47 commits into from
Sep 4, 2024
Merged

Release/v0.17.0 #169

merged 47 commits into from
Sep 4, 2024

Conversation

fivetran-catfritz
Copy link
Contributor

@fivetran-catfritz fivetran-catfritz commented Aug 30, 2024

@fivetran-catfritz fivetran-catfritz self-assigned this Aug 30, 2024
* MagicBot/documentation-updates

* Apply suggestions from code review

* Update README.md

Co-authored-by: fivetran-catfritz <111930712+fivetran-catfritz@users.noreply.github.com>

---------

Co-authored-by: fivetran-catfritz <111930712+fivetran-catfritz@users.noreply.github.com>
packages.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the effort here @fivetran-catfritz!

These changes look good to me. This is ready for release review and rollout!

Copy link
Contributor

@fivetran-reneeli fivetran-reneeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fivetran-catfritz ! Left some comments

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
- `chunk`: The text chunk itself
- `chunk_tokens_approximate`: Approximate token count for each segment
- This model is currently disabled by default. You may enable it by setting the `zendesk__unstructured_enabled` variable as `true` in your `dbt_project.yml`.
- This model was developed to limit the chunk sizes to approximately 5000 tokens for use with OpenAI, however you can change this limit by setting the variable `zendesk_max_tokens` in your `dbt_project.yml`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we only intending to use Zendesk NLP with Open AI which I feel is the language here? Or we want to be agnostic.

I would also reword it to the following:

This model was developed with the limit of chunk sizes to approximately 5000 tokens for use with OpenAI, however you can change this limit by setting the variable `zendesk_max_tokens` in your `dbt_project.yml`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! updated.

macros/count_tokens.sql Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug-report.yml Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature-request.yml Show resolved Hide resolved
CHANGELOG.md Outdated
## Under the hood
- Added integrity validations:
- Test to ensure `zendesk__sla_policies` and `zendesk__ticket_metrics` models produce consistent time results. ([#164](https://github.com/fivetran/dbt_zendesk/pull/164))
- Test to ensure `zendesk__ticket_metrics` contains all the tickets found in `stg_zendesk__ticket`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add PR link here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

fivetran-catfritz and others added 2 commits September 4, 2024 12:43
Co-authored-by: Renee Li <91097070+fivetran-reneeli@users.noreply.github.com>
Copy link
Contributor Author

@fivetran-catfritz fivetran-catfritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @fivetran-reneeli! I added comments inline to address your questions and also made the changelog updates you suggested. Let me know if it all looks good!

macros/count_tokens.sql Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature-request.yml Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug-report.yml Show resolved Hide resolved
CHANGELOG.md Outdated
- `chunk`: The text chunk itself
- `chunk_tokens_approximate`: Approximate token count for each segment
- This model is currently disabled by default. You may enable it by setting the `zendesk__unstructured_enabled` variable as `true` in your `dbt_project.yml`.
- This model was developed to limit the chunk sizes to approximately 5000 tokens for use with OpenAI, however you can change this limit by setting the variable `zendesk_max_tokens` in your `dbt_project.yml`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! updated.

CHANGELOG.md Outdated
## Under the hood
- Added integrity validations:
- Test to ensure `zendesk__sla_policies` and `zendesk__ticket_metrics` models produce consistent time results. ([#164](https://github.com/fivetran/dbt_zendesk/pull/164))
- Test to ensure `zendesk__ticket_metrics` contains all the tickets found in `stg_zendesk__ticket`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

Copy link
Contributor

@fivetran-reneeli fivetran-reneeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just reminder to switch the deps!

@fivetran-catfritz fivetran-catfritz merged commit 0b73c19 into main Sep 4, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants