feat: get diff between latest recommendation and cluster schema [PROD-1686] #103
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
taylorgaw
force-pushed
the
PROD-1686/get_diff
branch
from
88e815c to
9a6a51b
Compare
taylorgaw
changed the title
taylorgaw
requested review from
brandon-kaplan,
kartiknagappa,
romainissynced and
singhals
taylorgaw
force-pushed
the
PROD-1686/get_diff
branch
from
9a6a51b to
c0510ce
Compare
taylorgaw
force-pushed
the
PROD-1686/get_diff
branch
from
c0510ce to
138add5
Compare
singhals
requested changes
Mar 6, 2024
syncbrian
reviewed
Mar 7, 2024
moving methods to projects.py finalizing changes for tests
taylorgaw
requested review from
romainissynced,
singhals,
syncbrian and
taylorgaw
…code/syncsparkpy into PROD-1686/get_diff
singhals
requested changes
Mar 11, 2024
sync/models.py
Outdated
| spark_conf: Dict | ||
| spark_version: str | ||
| runtime_engine: str | ||
| aws_attributes: Dict |
Contributor
There was a problem hiding this comment.
aws_attributes in the azure config?
sync/api/projects.py
Outdated
| response_str = json.dumps(recommendation_response.result) | ||
| return Response( | ||
| result={ | ||
| "cluster_recommendations": json.loads(response_str), |
Contributor
There was a problem hiding this comment.
Its one recommendation, no? "cluster_recommendations" -> "cluster_recommendation"?
sync/api/projects.py
Outdated
| def get_updated_cluster_defintion( | ||
| project_id: str, cluster_spec_str: str | ||
| ) -> Response[Union[AWSProjectConfiguration, AzureProjectConfiguration]]: | ||
| """Print Cluster Definition merged with Project Configuration Recommendations. |
Contributor
There was a problem hiding this comment.
I think you meant "Return" and not "Print" here
sync/api/projects.py
Outdated
Comment on lines
467
to
479
| # Convert Response result object to str | ||
| latest_rec_str = json.dumps(rec_response.result) | ||
| # Convert json string to json | ||
| latest_recommendation = json.loads(latest_rec_str) | ||
| cluster_definition = json.loads(cluster_spec_str) | ||
| for key in latest_recommendation.keys(): | ||
| cluster_definition[key] = latest_recommendation[key] | ||
|
|
||
| # instance_source and driver_instance_source are not | ||
| # included in recommendation and need to be updated as well | ||
| driver_recommendation = cluster_definition["node_type_id"] | ||
| cluster_definition["instance_source"] = {"node_type_id": driver_recommendation} | ||
| cluster_definition["driver_instance_source"] = {"node_type_id": driver_recommendation} |
Contributor
There was a problem hiding this comment.
We should do a deep merge here like we do when we apply a recommendation from the library:
syncsparkpy/sync/_databricks.py
Line 633 in dc7a217
Contributor
There was a problem hiding this comment.
it might be good to review this pathway before merging this:
syncsparkpy/sync/_databricks.py
Line 524 in dc7a217
I think we are rewriting logic that exists already
taylorgaw
reviewed
Mar 11, 2024
| .PHONY: test | ||
| test: | ||
| pytest | ||
| pytest -vv |
Contributor
There was a problem hiding this comment.
This is consistent across our other repos for make test
singhals
approved these changes
Mar 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
We're adding functionality to display cluster recommendations, as well as a side by side comparison and a merged cluster definition.
Checklist
Before formally opening this PR, please adhere to the following standards:
Related Jira Ticket (PROD-1686)
Add any relevant testing examples or screenshots.