Validate dbt trigger_reason field to be less than 255 characters

[boraberke]

Validate and truncate trigger_reason field if it is longer than the limit of 255 characters.

closes: #34676

[boraberke]

Hey @josh-fell, would greatly appreciate your thoughts and suggestions on this!

[boraberke]

Hi @dirrao, did you have a chance to look at this PR again? Would appreciate your comments!

[josh-fell]

Nice work here!

I do think though that this logic should live in the DbtCloudHook instead. Moving the logic to the hook would cover both scenarios of folks using the operator and the DbtCloudHook.trigger_job_run() method explicitly (as part of a TaskFlow function for example).

[boraberke]

Nice work here!

I do think though that this logic should live in the DbtCloudHook instead. Moving the logic to the hook would cover both scenarios of folks using the operator and the DbtCloudHook.trigger_job_run() method explicitly (as part of a TaskFlow function for example).

Thanks for your review @josh-fell, I have updated accordingly!

I have a question about testing. I have only implemented a test for the DbtCloudHook, do you think we need to add a test to cover DbtCloudRunJobOperator trigger reason explicitly? I think the test for DbtCloudHook covers all the possible cases including operator and DbtCloudHook.trigger_job_run().

[josh-fell]

Nice work here!
I do think though that this logic should live in the DbtCloudHook instead. Moving the logic to the hook would cover both scenarios of folks using the operator and the DbtCloudHook.trigger_job_run() method explicitly (as part of a TaskFlow function for example).

Thanks for your review @josh-fell, I have updated accordingly!

I have a question about testing. I have only implemented a test for the DbtCloudHook, do you think we need to add a test to cover DbtCloudRunJobOperator trigger reason explicitly? I think the test for DbtCloudHook covers all the possible cases including operator and DbtCloudHook.trigger_job_run().

Agreed. Adding the test to DbtCloudHook is perfectly fine! There is another test which confirms passing the trigger_reason value from the operator to the hook already which is good enough.