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

Fix missing attributes from databricks_job data source #1798

Merged
merged 5 commits into from
Dec 2, 2022

Conversation

nkvuong
Copy link
Contributor

@nkvuong nkvuong commented Nov 25, 2022

This happens because we did not set expandTasks to the API when listing the jobs. For search by id, list without expandTasks and then re-search when job name is found

This closes #1797

@nkvuong nkvuong requested a review from alexott November 25, 2022 13:47
jobs/data_job.go Outdated
} else {
// if name is not provided, need to list all the jobs and search by id
// does not expand tasks to limit the results size
list, err = jobsAPI.List()
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have a job ID, then why not simply call Read using that ID? It will be more efficient :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a lot simpler now

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #1798 (634a2e1) into master (4a7ca53) will increase coverage by 0.30%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1798      +/-   ##
==========================================
+ Coverage   89.71%   90.02%   +0.30%     
==========================================
  Files         142      142              
  Lines       11222    11227       +5     
==========================================
+ Hits        10068    10107      +39     
+ Misses        743      712      -31     
+ Partials      411      408       -3     
Impacted Files Coverage Δ
jobs/data_job.go 93.54% <100.00%> (+5.08%) ⬆️
jobs/resource_job.go 95.74% <0.00%> (+0.85%) ⬆️
exporter/util.go 83.09% <0.00%> (+1.40%) ⬆️
exporter/importables.go 90.71% <0.00%> (+3.29%) ⬆️

Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

lgtm. Thank you for simplifying it

@nkvuong nkvuong force-pushed the fix/job_data_missing_attributes branch from 23345e2 to 634a2e1 Compare November 30, 2022 13:40
@nfx nfx enabled auto-merge (squash) December 1, 2022 09:15
@nfx nfx merged commit 93a035e into master Dec 2, 2022
@nfx nfx mentioned this pull request Dec 14, 2022
@nkvuong nkvuong deleted the fix/job_data_missing_attributes branch January 4, 2023 12:30
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this pull request Feb 15, 2023
…1798)

* re-add expand_tasks to retrieve settings

* fix error messages

* simplify read by Id
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.

[ISSUE] Issue with databricks_job Data Source
4 participants