josh-fell commented on PR #38896: URL: https://github.com/apache/airflow/pull/38896#issuecomment-2096042817
> > 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
