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

Followups on handling Photon eventlogs #953

Merged
merged 2 commits into from
Apr 23, 2024

Conversation

amahussein
Copy link
Collaborator

Signed-off-by: Ahmed Hussein (amahussein) a@ahussein.me

Fixes #948

  • this change makes the Qualification detect photon eventlogs fron environmentUpdate event
  • Adds a boolean that will be used later for applications with photon runtime in Prof/Qual tools
  • Moved some of the utilities to the new class DatabricksParseHelper
  • Fixed a bug in handling incorrect App parsing
  • Fixed unit tests

Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>

Fixes NVIDIA#948

- this change makes the Qualification detect photon eventlogs fron
  environmentUpdate event
- Adds a boolean that will be used later for applications with photon
  runtime in Prof/Qual tools
- Moved some of the utilities to the new class DatabricksParseHelper
- Fixed a bug in handling incorrect App parsing
- Fixed unit tests
@amahussein amahussein added bug Something isn't working core_tools Scope the core module (scala) labels Apr 19, 2024
@amahussein amahussein self-assigned this Apr 19, 2024
@amahussein amahussein requested a review from nartal1 April 19, 2024 19:33
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
Copy link
Collaborator Author

@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
I. addressed your review comments.

Copy link
Collaborator

@parthosa parthosa left a comment

Choose a reason for hiding this comment

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

Thanks @amahussein for this change. Minor typo nit. Removing the hardcoded strings was a good idea.


/**
* Checks if the properties indicate that the application is a Photon app.
* This ca be checked by looking for keywords in one of the keys defined in PHOTON_SPARK_PROPS
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: typo can

Copy link
Collaborator

@cindyyuanjiang cindyyuanjiang left a comment

Choose a reason for hiding this comment

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

Thanks @amahussein! LGTM.

@amahussein amahussein merged commit 79697ad into NVIDIA:dev Apr 23, 2024
15 checks passed
@amahussein amahussein deleted the spark-rapids-tools-948 branch April 23, 2024 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core_tools Scope the core module (scala)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Followups on handling Photon eventlogs
3 participants