Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Feature] Add
databricks_app
resource #4099[Feature] Add
databricks_app
resource #4099Changes from 9 commits
c8859e2
962d9f2
2e9137a
91b5166
bcf2a97
35b85eb
9c7ad60
863fa37
2d7b38c
b733ee9
ff8bee8
0daeea1
7449b45
eda8a91
f254176
18b3520
be96f91
8338d09
c6ee7ea
07ea8b7
3b1f6e2
9e41c69
0ad4294
8bdcda9
2bac7aa
17e1923
731469e
7dd91f4
14fea49
6bd77aa
25ae4ba
7ba55dd
a62a65f
08d4111
1628cb8
e30e677
b9f8e65
6e44632
29d73dd
ff11182
8af1c31
dac1bab
c4c9375
3e26f55
7336447
cfdec91
4b36d53
93763a8
37d3f1b
3b5015b
123432c
b84854a
6222b90
295c972
6a39e1b
e4aff40
c5a1181
2c3ed47
5b52d77
edc1d4c
30398da
560ce27
687996c
5779ece
b3634bd
32cba97
4fdd72b
0619e2e
a9f12b6
2feedf4
4feb053
d10447f
ed7246e
0d49760
a0297e6
adb9746
5adec2e
6496607
285c29f
ed7b0ec
853622c
9588149
07a77bc
c3ef933
5da3a69
ecd7d29
f138239
b929583
5c4a1c7
ec950fb
eaf71eb
7f8a845
d16ec81
29aa7e9
7c539b8
e282b55
223acf8
92dbbe9
3ba9694
8d42b08
f0117c8
28c6acc
361cec4
402f40a
c425508
fc9e4c8
10c456e
86b8cce
5f107cf
c414f8a
a9e438c
66fe1b9
59e16d5
e0dd84d
9114ea1
52e975c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rauchy Can you discuss with @nkvuong about adding this alias to our API spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resource types are mutually exclusive per resource element (
secret
,sql_warehouse
, etc)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can add multiple resources to an app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but within a resource you should only have one of (
secret
,sql_warehouse
, etc).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add this constraint back, where you cannot have more than one defined per resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the type is nested within
resource
, we cannot add constraints with sdkv2 - would need to rewrite this to framework to be able to add this validation ruleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's at least do the validation in the create/update. (Or at least test to verify what its behavior is.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW: Can we use the plugin framework for this new resource? Generally speaking, new resources should use the plugin framework, since it should be much more consistent for handling things like effective fields, dealing with lists, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we discussed it offline and agreed that the deployment won't be part of TF configuration. Instead, if an users want to deploy, they use CLI to do that. Otherwise it's confusing because we do the deployments only when source_code_path changed but we need to do deployments when source code is changed.
@pietern wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update this to get it back into a healthy state? if not this is fine. If so, let's set the ID before waiting for ready, then subsequent applies can call update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above, I think we should remove app deployment altogether for now