Adding files that are required for jupyterlab submit button extension… by TianshuLuan-UofT · Pull Request #7986 · MarkUsProject/Markus

TianshuLuan-UofT · 2026-06-03T16:02:19Z

Linked to pull request at MarkUsProject/markus-jupyter-extension#3

Proposed Changes

Added necessary files for the JupyterLab extension (submit button) to work in the MarkUs server

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)	x
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

[ x] I have performed a self-review of my changes.
- Check that all changed files included in this pull request are intentional changes.
- Check that all changes are relevant to the purpose of this pull request, as described above.
I have added tests for my changes, if applicable.
- This is required for all bug fixes and new features.
I have updated the project documentation, if applicable.
- This is required for new features.
If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

I have updated the project Changelog (this is required for all changes).
I have verified that the pre-commit.ci checks have passed.
I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported by Coveralls.
I have requested a review from a project maintainer.

Questions and Comments

(Include any questions or comments you have regarding your changes.)

…. Linked to pull request at MarkUsProject/markus-jupyter-extension#3

for more information, see https://pre-commit.ci

coveralls · 2026-06-03T16:27:49Z

Coverage Report for CI Build 26897103316

Coverage decreased (-0.2%) to 90.005%

Details

Coverage decreased (-0.2%) from the base build.
Patch coverage: 111 uncovered changes across 1 file (27 of 138 lines covered, 19.57%).
No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
app/controllers/api/jupyter_submissions_controller.rb	134	23	17.16%
Total (3 files)	138	27	19.57%

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	50218
Covered Lines:	46179
Line Coverage:	91.96%
Relevant Branches:	2226
Covered Branches:	1023
Branch Coverage:	45.96%
Branches in Coverage %:	Yes
Coverage Strength:	122.37 hits per line

💛 - Coveralls

david-yz-liu · 2026-06-10T20:27:02Z

+
+require 'json'
+
+module Api


About naming and organization:

The new route and controller method should be under a new namespace. I suggest app/controllers/jupyter/submissions_controller.rb, with a corresponding route under the jupyter (not api).

I suggest calling the route submit rather than create (though it can still be a POST action).

david-yz-liu · 2026-06-10T20:27:56Z

+    skip_verify_authorized only: :create
+
+    def create
+      payload = request.request_parameters.presence || params.to_unsafe_h


Don't do this; use params

david-yz-liu · 2026-06-10T20:29:44Z

 # Handle Cross-Origin Resource Sharing (CORS) in order to accept cross-origin Ajax requests.

 # Read more: https://github.com/cyu/rack-cors
+# frozen_string_literal: true


Delete this

david-yz-liu · 2026-06-10T20:30:37Z

-    # Add hosts running jupyterhub to the Settings.jupyter_server.hosts settings option.
-    origins(*Settings.jupyter_server.hosts)
-    resource %r{/api/courses/\d+/assignments/\d+/submit_file},
+    origins 'http://localhost:8889', 'http://127.0.0.1:8889'


This should not be changed. Instead you can change the actual setting value, which is stored in settings.yml

david-yz-liu · 2026-06-10T20:32:40Z

@@ -0,0 +1,102 @@
+# frozen_string_literal: true
+
+require 'tempfile'


This require is unnecessary (Tempfile is already loaded and accessible)

david-yz-liu · 2026-06-10T21:01:22Z

+            'Could not find or create a grouping for this student and assignment.'
+    end
+
+    def notebook_content_as_string(content)


This is unnecessary (and somewhat overly restrictive). Whatever file data is returned from JupyterHub should be streamed directly into a tempfile that's stored as a submitted file in MarkUs. No parsing is necessary, and in fact undesirable. In the future we may wish to allow other types of files to be submitted to MarkUs from JupyterHub.

david-yz-liu · 2026-06-10T21:01:33Z

+
+      raise ArgumentError, 'Destination filename is missing.' if filename.blank?
+
+      unless filename.end_with?('.ipynb')


remove this check (see my above comment)

david-yz-liu · 2026-06-10T21:01:49Z

+    # SubmissionsHelper#upload_file expects this helper method to exist.
+    # MainApiController has it, but this controller inherits from ApplicationController
+    # to avoid the MarkUs API permission layer for the local prototype.
+    def has_missing_params?(required_params)


Delete this method (see my above comment about not using upload_file

david-yz-liu · 2026-06-10T21:03:02Z

+  end
+
+  def hub_user_uri
+    hub_origin = ENV.fetch('JUPYTERHUB_API_ORIGIN', nil).presence || @origin


Don't using an environment variable; instead, create a new setting (under the jupyter_server namespace)

david-yz-liu · 2026-06-10T21:03:25Z

+  end
+
+  def normalize_origin(origin)
+    overridden = ENV.fetch('JUPYTER_FETCH_ORIGIN', nil)


See above comment (though right now I'm not sure why there are two different environment variables)

Tianshu Luan and others added 2 commits June 3, 2026 11:46

Adding files that are required for jupyterlab submit button extension…

305c82b

…. Linked to pull request at MarkUsProject/markus-jupyter-extension#3

[pre-commit.ci] auto fixes from pre-commit.com hooks

b39bb0c

for more information, see https://pre-commit.ci

david-yz-liu reviewed Jun 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding files that are required for jupyterlab submit button extension…#7986

Adding files that are required for jupyterlab submit button extension…#7986
TianshuLuan-UofT wants to merge 2 commits into
MarkUsProject:masterfrom
TianshuLuan-UofT:submit_button

TianshuLuan-UofT commented Jun 3, 2026

Uh oh!

coveralls commented Jun 3, 2026 •

edited

Loading

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

david-yz-liu Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,102 @@
		# frozen_string_literal: true

		require 'tempfile'


		raise ArgumentError, 'Destination filename is missing.' if filename.blank?

		unless filename.end_with?('.ipynb')

Conversation

TianshuLuan-UofT commented Jun 3, 2026

Proposed Changes

Type of Change

Checklist

Questions and Comments

Uh oh!

coveralls commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 26897103316

Coverage decreased (-0.2%) to 90.005%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Jun 3, 2026 •

edited

Loading