Issue #127 - Respect SQL Servers Parameter Limit

[jacobm001]

Fixes: #127

Changes

• The batch_size parameter is now max_batch_size in the macro sqlserver__basic_load_csv_rows()
• The macro calc_batch_size() will now return the max_batch_size if it won't exceed SQL Server's parameter limit. Otherwise it returns the maximum size that can run without erroring.

[jacobm001]

The AzureSQL test failed with an error message suggesting that the test harness is at fault.

Get Token request returned http error: 401 and server response: {"error":"user_password_expired","error_description":"AADSTS50055: The password is expired.\r\nTrace ID: 76d52eea-678e-4005-b537-99184ed16301\r\nCorrelation ID: fa068f34-9802-472c-af90-3c8acc7474d8\r\nTimestamp: 2021-07-15 02:45:34Z","error_codes":[50055],"timestamp":"2021-07-15 02:45:34Z","trace_id":"76d52eea-678e-4005-b537-99184ed16301","correlation_id":"fa068f34-9802-472c-af90-3c8acc7474d8","error_uri":"https://login.microsoftonline.com/error?code=50055","suberror":"user_password_expired","password_change_url":"https://portal.microsoftonline.com/ChangePassword.aspx"}

[jacobm001]

@dbt-msft/dbt-sqlserver-core Any chance we can get the credentials updated for Azure?

[dataders]

@dbt-msft/dbt-sqlserver-core Any chance we can get the credentials updated for Azure?

done!

[dataders]

@jacobm001 thanks for closing out #127 for us. You'll have made @hz-lschick's day I'm sure.

The logic is great -- my only comment is that currently this won't affect end users, unless they are working off this adapter directly and modify the 200 below to be a bigger number.

{% macro sqlserver__load_csv_rows(model, agate_table) %}
  {{ return(sqlserver__basic_load_csv_rows(model, 200, agate_table) )}}
{% endmacro %}

I propose doing two things:

1. making the batch_size argument passed to sqlserver__basic_load_csv_rows to be a variable with a default batch size. This would allow users to increase the batch size if they wish in their dbt_project.yml

{% macro sqlserver__load_csv_rows(model, agate_table) %}
  {% set max_batch_size = var("max_batch_size", 200) %}
  {{ return(sqlserver__basic_load_csv_rows(model, max_batch_size, agate_table) )}}
{% endmacro %}

# dbt_project.yml
vars:
  max_batch_size: 400  # or whatever they want?

2. Perhaps we increase the default max_batch_size to be a number greater than 200 because your new logic will fix things anyway?

[jacobm001]

I agree. I actually left it this way because I thought we might want to make this available to users, but wasn't sure how the rest of the team felt about it. I've added the logic and set the default to 400 as suggested.

[jacobm001]

Sorry for the last final push. I realized that now this is configurable we needed some documentation.

[dataders]

@jacobm001 no worries! My last ask is that you edit the CHANGELOG.md to reference this change. I'm going to publish a pre-release version of v0.20.0 later today. If you can get the changelog updated, I can get this change into the pre-release (and thus also into v0.20.0)

[jacobm001]

@swanderz: I think it makes more sense to merge this in and then have you make the alteration to the changelog in the 0.20.0 PR you have going. There's no way for me to edit the changelog without creating a conflict... I could merge your branch into mine, but that feels rather messy...

Happy to write you the text here if that helps.

[dataders]

@swanderz: I think it makes more sense to merge this in and then have you make the alteration to the changelog in the 0.20.0 PR you have going. There's no way for me to edit the changelog without creating a conflict... I could merge your branch into mine, but that feels rather messy...

Happy to write you the text here if that helps.

I'll handle to weirdness -- don't worry! just make your changes in this branch, and I'll handle it after. Thanks again for your contribution -- glad to have us all on the same "team" now.

[dataders]

thanks again @jacobm001!