Fix node_type data source by skipping node types that aren't available from cloud provider #1534
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1534 +/- ##
==========================================
+ Coverage 90.17% 90.18% +0.01%
==========================================
Files 132 132
Lines 10560 10567 +7
==========================================
+ Hits 9522 9530 +8
+ Misses 663 662 -1
Partials 375 375
|
alexott
force-pushed
the
issue-1533-fix
branch
from
33613fb
to
37274c8
Compare
clusters/data_node_type.go
Outdated
| if nt.NodeInfo != nil { | ||
| for _, st := range nt.NodeInfo.Status { | ||
| if st == CloudProviderNodeStatusNotAvailableInRegion || st == CloudProviderNodeStatusNotEnabled { | ||
| shouldBeSkipped = true | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
should we also check node_info.available_core_quota >= num_cores?
There was a problem hiding this comment.
not sure if it will help a lot, as we're working on the individual node level...
There was a problem hiding this comment.
I suppose it depends on the use-case. For mounts, it will help since we only start one node always and use it for all mounts. When the use-case is to for user-defined clusters, it's less clear how much it would help since the number of cluster and number of executors is unknown at this stage.
There was a problem hiding this comment.
comment doesn't block merge
alexott
force-pushed
the
issue-1533-fix
branch
from
37274c8
to
327b7a6
Compare
clusters/data_node_type.go
Outdated
| @@ -89,6 +94,18 @@ type NodeType struct { | |||
| Graviton bool `json:"is_graviton,omitempty"` | |||
| } | |||
|
|
|||
| func (nt NodeType) shouldBeSkipped() bool { | |||
| if nt.NodeInfo != nil { | |||
There was a problem hiding this comment.
can we further simplify it to if nt.NodeInfo == nil return false?
alexott
force-pushed
the
issue-1533-fix
branch
from
327b7a6
to
bff0f46
Compare
…in subscription/region (databricks#1534)
Also, fixed tests that use azure-cli - Go stopped to execute programs when PATH has relative paths
This fixes #1533