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

[notebook] fix notebook saving type #3373

Closed

Conversation

sumtumn
Copy link

@sumtumn sumtumn commented Jul 3, 2023

…ppet's type

What changes were proposed in this pull request?

How was this patch tested?

  • (Please explain how this patch was tested. Ex: unit tests, manual tests)
    manual tests

Please review Hue Contributing Guide before opening a pull request.

@bjornalm bjornalm requested review from agl29 and Harshg999 July 3, 2023 13:47
@bjornalm
Copy link
Collaborator

bjornalm commented Jul 3, 2023

@sumtumn Thanks for the PR! We will review it shortly.

@amitsrivastava
Copy link
Collaborator

@sumtumn Can you please describe the issue in more detail? What value are you seeing without this fix vs what is expected. This helps in verifying the fix. Thanks.

Also, please shorten the title of you commit to avoid it getting cutoff.

@sumtumn
Copy link
Author

sumtumn commented Aug 10, 2023

@amitsrivastava
How to reproduce this:

  1. Open notebook Editor
  2. Add one or more snippet types (Hive, Scala, etc)
  3. Save as Document
  4. Open Document Browser
  5. Load saved Notebook Document
  6. The Notebook shows up, but does not allow the user to add more snippets to it or edit it outside of changing the existing queries.

Because the notebook type changed to be the first snippet's dialect while saveing notebook, e.g. the first snippet of the saved notebook was hive, then the saved notebook type was chaged to query-hive. This makes we can't add more snippets to the saved notebook. And it's also inappropriate because there are many different types of snippets in the same notebook, like hive, spark.

@sumtumn sumtumn force-pushed the fix-notebook-save-wrong-type branch from 30ff67f to 2f60217 Compare August 10, 2023 11:57
 * [notebook] use notebook type instead of the first snippet's type
@sumtumn sumtumn force-pushed the fix-notebook-save-wrong-type branch from 2f60217 to abd4ce6 Compare August 10, 2023 12:24
@sumtumn sumtumn changed the title [notebook] fix notebook saving notebook type instead of the first sni… [notebook] fix notebook saving type Aug 10, 2023
@bjornalm
Copy link
Collaborator

@amitsrivastava @Harshg999 @agl29 can we get som eyes on this PR?

Copy link
Collaborator

@amitsrivastava amitsrivastava 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 @sumtumn

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Oct 10, 2023
@github-actions github-actions bot closed this Oct 20, 2023
@bjornalm bjornalm reopened this Oct 20, 2023
@bjornalm
Copy link
Collaborator

@Harshg999 Can you have a quick look and maybe we can merge this one?

@github-actions github-actions bot removed the Stale label Oct 21, 2023
Copy link

github-actions bot commented Dec 5, 2023

This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Dec 5, 2023
@github-actions github-actions bot closed this Dec 15, 2023
@risyomei
Copy link

I had the same issue on CDP 7.2.17's hue. resolved it with this PR.
@sumtumn Thanks man.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants