Skip to content

Conversation

@gorskysd
Copy link
Contributor

@gorskysd gorskysd commented Nov 9, 2023

https://synccomputing.atlassian.net/browse/PROD-1307

Upgrade to the cluster monitoring for both aws-databricks and azure-databricks. For each instance that is added to the cluster (defined as having the cluster tag), the instances_timeline list keeps track of the continuous intervals where those instances first and last seen running.

I took this opportunity to reshape the cluster report payload to reduce the unnecessary double-nesting of some fields. Although this wasn't strictly necessarily, it rather significantly reduces the complexity of processing in multiple areas both within syncsparkpy and in the backend. Accommodating this change in the backend to maintain backwards compatibility was straightforward and will be well tested.

Corresponding backend PR: https://github.com/synccomputingcode/sync_backend/pull/445

Slide deck with testing results: https://docs.google.com/presentation/d/1x05FDH3xRkANAo0PchKeBUu9ZFxicHFP3m11dnhxqj0/edit#slide=id.g29bb9035aeb_0_71

@gorskysd gorskysd marked this pull request as ready for review November 16, 2023 03:58
@gorskysd gorskysd changed the title Bugfix/prod 1307 instance timeline upgrade PROD-1307 Timeline upgrade (library) Nov 21, 2023
volumes_response = Response(error=DatabricksError(message=missing_message("ebs volumes")))
else:
volumes_response = Response(result={"Volumes": cluster_info.get("Volumes")})
volumes_response = Response(result=cluster_info.get("volumes"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to make confirm the capitalization change of "volume" here is expected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep this is expected, that was part of the "de-nesting" that I talked about.

Copy link
Contributor

@syncbrian syncbrian left a comment

Choose a reason for hiding this comment

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

LGTM!

@gorskysd gorskysd merged commit 1a4515a into main Nov 28, 2023
@gorskysd gorskysd deleted the bugfix/PROD-1307-instance-timeline-upgrade branch November 28, 2023 14:47
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.

3 participants