Skip to content

Remove "RTC" drive prefix from filepath added by jupyter-collaboration when using notebook scheduler widget #577

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

Merged
merged 2 commits into from
Mar 20, 2025

Conversation

asmita-sharma1625
Copy link
Contributor

@asmita-sharma1625 asmita-sharma1625 commented Mar 11, 2025

When creating jobs using the job creation widget in the notebook top panel, check and remove "RTC" drive prefix from the current notebook file path which is added by jupyter-collaboration.

References

Reproduce

In your conda environment for Jupyter Labs,
Install and enable Jupyter Collaboration Extension

pip install jupyter_collaboration
jupyter server extension enable jupyter_collaboration

Install and enable Jupyter Scheduler Extension

pip install jupyter_scheduler
jupyter server extension enable jupyter_scheduler

Run Jupyter Labs

jupyter lab

In the local jupyter lab server running on your browser, perform the following steps manually:

  • Create a Job using the Job Creation Widget in the notebook top panel ✅
  • Create a Job Definition (job with a schedule) ✅
  • Create a job or a job definition with input folder included ✅
  • Job creation will fail with the error - Input path 'RTC:jupyter-scheduler/rtc-path-test/test-rtc.ipynb' does not exist.
Screenshot 2025-03-12 at 2 52 40 AM

Code changes

Command createJobCurrentNotebook now uses getSelectedFilePath and getSelectedFileBaseName helper functions which use serviceManager.contents API to check if there is an added “RTC” drive name in the file path and return local path without it.

User-facing changes

  • While using both jupyter-collaboration and jupyter-scheduler extensions, the error Input path 'RTC:jupyter-scheduler/rtc-path-test/test-rtc.ipynb' does not exist. will no longer surface during job creation using the notebook top panel widget.
  • The input file value will display the file path without the "RTC:" prefix.
  • Note: The "RTC:" prefix displayed in launcher tab is not scoped in this PR.

Here’s the video showcasing the fixed experience
https://github.com/user-attachments/assets/86cb5cf0-7e98-4681-86e8-ff847bf23e57

Backwards-incompatible changes

None.

@asmita-sharma1625 asmita-sharma1625 changed the title Remove "RTC" drive prefix from notebook file path added by the jupyter-collaboration when using notebook scheduler widget Remove "RTC" drive prefix from filepath added by jupyter-collaboration when using notebook scheduler widget Mar 11, 2025
@andrii-i andrii-i added the bug Something isn't working label Mar 12, 2025
@andrii-i andrii-i modified the milestone: 2.11.0 Mar 12, 2025
Copy link
Collaborator

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Hi @asmita-sharma1625. Thank you for your work. I see that you noted that file name + file extension is used for the Job Name when job is created from the notebook top panel vs file name only when created from file browser and changed it. This makes sense as we already have a File Name field for file name + file extension, behavior across creation from file browser and top notebook panel should be consistent.

Taking that into account, to fix E2E test could you please manually update the expected snapshot used in the failed E2E test by replacing it with actual snapshot for that test from the failed E2E run report? Expected snapshots are stored in ui-tests/tests/jupyter_scheduler.spec.ts-snapshots folder.

@asmita-sharma1625
Copy link
Contributor Author

Hi @asmita-sharma1625. Thank you for your work. I see that you noted that file name + file extension is used for the Job Name when job is created from the notebook top panel vs file name only when created from file browser and changed it. This makes sense as we already have a File Name field for file name + file extension, behavior across creation from file browser and top notebook panel should be consistent.

Taking that into account, to fix E2E test could you please manually update the expected snapshot used in the failed E2E test by replacing it with actual snapshot for that test from the failed E2E run report? Expected snapshots are stored in ui-tests/tests/jupyter_scheduler.spec.ts-snapshots folder.

Thanks for the confirmation on the expected job name. Updating accordingly.

@asmita-sharma1625
Copy link
Contributor Author

Fixed UI tests as discussed. Kindly review or merge, if not further comments.

@asmita-sharma1625
Copy link
Contributor Author

Suggestion - I am not sure if the job creation page from file browser or the top panel widget should differ in any scenario. If not, we can possibly refer to the same snapshot for both the UI test cases. This will help contributors identify failures in the other workflow while modifying one workflow, keeping the user experience in sync.

I can do the needful if the suggestion seems helpful.

@andrii-i
Copy link
Collaborator

andrii-i commented Mar 17, 2025

@asmita-sharma1625 thank you for the update. Everything looks good.

Suggestion - I am not sure if the job creation page from file browser or the top panel widget should differ in any scenario. If not, we can possibly refer to the same snapshot for both the UI test cases. This will help contributors identify failures in the other workflow while modifying one workflow, keeping the user experience in sync.

I can do the needful if the suggestion seems helpful.

Job creation page in both scenarios should look the same (given the same inputs and settings). This suggestion makes a lot of sense and would be a clear improvement to the E2E tests, let's implement it. Thank you for proactively making an improvement suggestion and volunteering to implement it.

@asmita-sharma1625
Copy link
Contributor Author

@asmita-sharma1625 thank you for the update. Everything looks good.

Suggestion - I am not sure if the job creation page from file browser or the top panel widget should differ in any scenario. If not, we can possibly refer to the same snapshot for both the UI test cases. This will help contributors identify failures in the other workflow while modifying one workflow, keeping the user experience in sync.
I can do the needful if the suggestion seems helpful.

Job creation page in both scenarios should look the same (given the same inputs and settings). This suggestion makes a lot of sense and would be a clear improvement to the E2E tests, let's implement it. Thank you for proactively making an improvement suggestion and volunteering to implement it.

Updated and added the commit to this PR. It was also mentioned as a TODO in UI tests to sync the job names for job creation across file browser and toolbar workflows. This TODO is also resolved with the earlier commit.

Copy link
Collaborator

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Everything looks good. Thank you for going beyond the issue description and making additional improvements.

@andrii-i andrii-i merged commit 6b79522 into jupyter-server:main Mar 20, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When job is created from the notebook top panel, input path with "RTC:" still breaks functionality
2 participants