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

Refactor Databricks-AWS Qual tool to cache and process pricing info from DB website #1141

Merged
merged 5 commits into from
Jun 28, 2024

Conversation

cindyyuanjiang
Copy link
Collaborator

Fixes #1139.

Changes

  • Refactored DatabricksAWSPriceProvider to cache and use pricing info from DB official website instead of outdated static file
  • Removed now stale databricks-premium-catalog.json file under resources
  • Fixed a previous DBU cost calculation bug

Testing

spark_rapids qualification --eventlogs <my-event-logs> --platform databricks-aws --cluster <my-cluster-props>

Run the cmd above and confirm the pricing calculation is the same before and after this PR.

Signed-off-by: cindyyuanjiang <cindyj@nvidia.com>
Signed-off-by: cindyyuanjiang <cindyj@nvidia.com>
Signed-off-by: cindyyuanjiang <cindyj@nvidia.com>
@cindyyuanjiang cindyyuanjiang self-assigned this Jun 26, 2024
@cindyyuanjiang cindyyuanjiang added feature request New feature or request user_tools Scope the wrapper module running CSP, QualX, and reports (python) labels Jun 26, 2024
Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang !
That's good that we have finally a catalog file to use for databricks.

Have you tested the fat-mode build? We need to make sure that these changes work fine when the tools are running offline.

@@ -0,0 +1,71 @@
# Copyright (c) 2023, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

2024 copyrights.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed, thanks!

Signed-off-by: cindyyuanjiang <cindyj@nvidia.com>
@cindyyuanjiang
Copy link
Collaborator Author

Have you tested the fat-mode build? We need to make sure that these changes work fine when the tools are running offline.

Thanks @amahussein! Tested the fat-mode build successfully.

Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @cindyyuanjiang

Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

@cindyyuanjiang
General comment on styling: we are trying to enforce defining the return type of each function as much as possible.
This is going to hunt us back with pylint moving forward which will cause the code to fail all pylint checks. For function returning nothing, then it is recommended to define it as def foo() -> None:

Signed-off-by: cindyyuanjiang <cindyj@nvidia.com>
@cindyyuanjiang
Copy link
Collaborator Author

@cindyyuanjiang General comment on styling: we are trying to enforce defining the return type of each function as much as possible. This is going to hunt us back with pylint moving forward which will cause the code to fail all pylint checks. For function returning nothing, then it is recommended to define it as def foo() -> None:

Thanks @amahussein! Updated function return types.

Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang !
LGTME

@cindyyuanjiang cindyyuanjiang merged commit 93e512e into NVIDIA:dev Jun 28, 2024
14 checks passed
@cindyyuanjiang cindyyuanjiang deleted the spark-rapids-tools-1139 branch June 28, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request user_tools Scope the wrapper module running CSP, QualX, and reports (python)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor DB AWS qual tool to cache and process pricing info from DB website
2 participants