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.

Sign up for GitHub

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

Jump to bottom

telemetry/add kedro_Viz_version to heaps #2194

Merged
merged 9 commits into from
Nov 19, 2024
Merged

telemetry/add kedro_Viz_version to heaps #2194

merged 9 commits into from
Nov 19, 2024
+10 −2

Conversation

Huongg
Copy link
Contributor

@Huongg Huongg commented Nov 15, 2024

Description

Fixes kedro-org/kedro-plugins#923

To include kedro_viz_version when user interacts with viz

Development notes

When test this locally, ensure you enable consent: true in project-demo telemetry, and in the heap dash board development environment, it should show kedro_viz_version as one of the property
Screenshot 2024-11-15 at 13 17 39

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

add kedro_Viz_version to heaps
cf34990 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
@Huongg Huongg self-assigned this Nov 15, 2024
update release note
9c96352 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
@Huongg Huongg requested a review from jitu5 November 15, 2024 13:26
@Huongg Huongg changed the title add kedro_Viz_version to heaps telemetry/add kedro_Viz_version to heaps Nov 15, 2024
rashidakanchwala
rashidakanchwala reviewed Nov 18, 2024
View reviewed changes
RELEASE.md Outdated
@@ -24,6 +24,7 @@ Please follow the established format:
- Refactor `DatasetStatsHook` to avoid showing error when dataset doesn't have file size info (#2174)
- Fix 404 error when accessing the experiment tracking page on the demo site (#2179)
- Add check for port availability before starting Kedro Viz to prevent unintended browser redirects when the port is already in use (#2176)
- Include kedro_viz_version in telemetry. (#2194)
Copy link
Contributor

Choose a reason for hiding this comment

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

Include Kedro Viz version in telemetry.

Huong Nguyen added 3 commits November 18, 2024 11:01
update release note and lint error
f8c1e33 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
Fix lint error
1e3df5f 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
Merge remote-tracking branch 'origin' into telemetry/kedro-viz-version
14cb1e7
@rashidakanchwala
Copy link
Contributor

i cannot checkout this branch @jitu5 , can you ??

@jitu5
Copy link
Contributor

jitu5 commented Nov 18, 2024

i cannot checkout this branch @jitu5 , can you ??

@rashidakanchwala Yes I can.

@jitu5
Copy link
Contributor

jitu5 commented Nov 18, 2024

Hey @Huongg

I tested locally and here as well https://heapanalytics.com/app/env/2388822444/data-galaxy?view=live-data-feed
but I cant see kedro-viz version on heap data for both CLI and UI event
Screenshot 2024-11-18 at 1 01 29 p m Screenshot 2024-11-18 at 1 01 18 p m

@rashidakanchwala
Copy link
Contributor

@jitu5 , u r on prod environment, u should check dev environment

@Huongg
Copy link
Contributor Author

Huongg commented Nov 18, 2024

hey @jitu5 let me know if you can see it in the development environment. Im happy to jump on the call to test this together. This is what I currently see from my side

Screenshot 2024-11-18 at 13 25 48

jitu5
jitu5 approved these changes Nov 18, 2024
View reviewed changes
Copy link
Contributor

@jitu5 jitu5 left a comment

Choose a reason for hiding this comment

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

Thanks @Huongg

rashidakanchwala
rashidakanchwala approved these changes Nov 18, 2024
View reviewed changes
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

i have had some difficulties checking out this branch. but since Jitendra tested and approved. I will approve as well as code wise it looks fine.

set version in base.py
6427e3e 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
rashidakanchwala
rashidakanchwala requested changes Nov 18, 2024
View reviewed changes
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

Yup. tried it . not working for me as well

revert back to the old syntax
a002b8e 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
@Huongg
Copy link
Contributor Author

Huongg commented Nov 19, 2024

hey both I just pushed the change and tested again and its working from my side. Just tested with @rashidakanchwala too, can you confirm its okay now?

rashidakanchwala
rashidakanchwala approved these changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

thanks Huong!

@Huongg Huongg merged commit ca734e4 into main Nov 19, 2024
25 checks passed
@Huongg Huongg deleted the telemetry/kedro-viz-version branch November 19, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jitu5 jitu5 jitu5 approved these changes

@rashidakanchwala rashidakanchwala rashidakanchwala approved these changes

@ravi-kumar-pilla ravi-kumar-pilla Awaiting requested review from ravi-kumar-pilla ravi-kumar-pilla is a code owner

@astrojuanlu astrojuanlu Awaiting requested review from astrojuanlu astrojuanlu is a code owner

Assignees

@Huongg Huongg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Telemetry: tracking kedro-viz version
3 participants
@Huongg @rashidakanchwala @jitu5