JenspederM / kedro-databricks

A Databricks Plugin for Kedro
MIT License
13 stars 5 forks source link

feat(bundle): add conf parameter to the generate_resources method #53

Closed npfp closed 1 month ago

npfp commented 2 months ago

This way the conf-source param of the tasks points to the right location.

codeclimate[bot] commented 2 months ago

Code Climate has analyzed commit 43b40cb1 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

View more on Code Climate.

npfp commented 2 months ago

My mistake, this change is required to make #52 fully working.

npfp commented 1 month ago

Hello @JenspederM,

Have you had a chance to review it? I believe the tests are failing because of lint being picker that in the previous commit.

JenspederM commented 1 month ago

Hey @npfp,

I have reviewed, and there is essentially nothing wrong with your or. There is however a problem with CI that I currently don't know how to solve.

I will have to spent a bit of time on that, but can merge this in if it's time-critical? 😊

JenspederM commented 1 month ago

Maybe you know... so the issue is that basically all tests require access to a Databricks workspace and therefore auth secrets.

Repository secrets are not available when external contributors make PRs without the use of the pull_request_target trigger. However, with this trigger, tests and linting is made against the target branch, main, and not the PR branch meaning that they always lag behind...

astrojuanlu commented 1 month ago

Maybe you know... so the issue is that basically all tests require access to a Databricks workspace and therefore auth secrets.

Perhaps PRs should mostly run unit tests, and let integration tests be run on a nightly job against the main branch? Some bugs might slip, but the contributor experience could be improved.

npfp commented 1 month ago

Ok thx for the explanation. I'll look into it.

JenspederM commented 1 month ago

Hey @npfp

I resolved the issues regarding testing. And while I was at it, I concorporated these changes. Hope that's okay :)

npfp commented 1 month ago

That's great, thx a lot!