libretexts / ngshare Goto Github PK

API modification: checksum is md5sum, whenever list_only or not return checksum
Database modification: checksum for files
Backend: ...

Currently instructors are not able to fetch and list submissions from instructors in ngshare. Instructors should be able to do this.

Once proper user authentication is supported, the student ID can be dropped from POST /api/submission/<course_id>/<assignment_id>/<student_id>. The student ID shall be determined from the user's credentials. The request will become POST /api/submission/<course_id>/<assignment_id>.

Allow add multiple students in one API call

When ngshare creates a datetime object, the timezone information is missing. This causes the timestamp string to have no timezone (for example, "2020-01-30 10:30:47.524219 " instead of "2020-01-30 10:30:47.524219 UTC").
https://github.com/lxylxy123456/ngshare/blob/d3438c7bb486cf3a2794020379ad24db547a281c/ngshare/database/database.py#L171
Maybe we can use this instead.

        self.timestamp = datetime.datetime.now(datetime.timezone.utc)

Hello,
We are trying to install ngshare, but helm install ngshare helmchart/ngshare returns an error that helm install can only accept one argument.

Also, after changing the directory to helmchart and running helm install ngshare we get this error:

Error: parse error in "ngshare/templates/_helpers.tpl": template: ngshare/templates/_helpers.tpl:58: function "get" not defined

We need to write documentation and modify ngshare to include support for adding students and instructors to a course

What happens when an instructor upload feedbacks for a same submission? Should the files be overwritten?

The GET /api/submissions/<course_id>/<assignment_id>/<student_id> request fails with sqlalchemy.exc.OperationalError. The reason is "no such column: submissions.random".

https://github.com/lxylxy123456/ngshare/blob/be15bd3d0646463693c7861942c89bf9cd5a5842/ngshare/database/database.py#L111

Why is not Submission.student a mapping to student? (currently it is TEXT of student id)

@rkevin-arch can you add something to README.md to explain how to use the testing directory?

in this section: https://github.com/lxylxy123456/ngshare#installation-and-setup

For redundancy we may want to add "student_id" here:

https://github.com/lxylxy123456/ngshare/blob/master/api-specifications.md#response-4

We may need to mock a Jupyter hub service to be able to write test cases (testing vserver is easy: we just need to send a username; but testing ngshare requires logging in etc, and I don't want to have )

It seems that JupyterHub is testing, but nbgrader is not testing its server_extensions?

Useful links:

• https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/tests/conftest.py
• https://github.com/jupyterhub/jupyterhub/blob/master/CONTRIBUTING.md#testing

Store file size in db

I think that we should transfer this repo to https://github.com/libretexts before school lets out. This will help keep it in our purview to maintain for the future.

@rkevin-arch I think you proposed this a long time ago and I rejected. Now I changed my mind and think we should make vngshare available by python3 ngshare.py --vngshare ....

My reasoning is that users cannot easily start vngshare from a installed pip package. If we do not merge then they may need to do:
python3 -c 'import ngshare; ngshare.vngshare.main()' --host 127.0.0.2

From meeting

We need to include the random str in the responses of /api/submissions/<course_id>/<assignment_id>/<student_id> and GET /api/submissions/<course_id>/<assignment_id>

As discussed in #10, use uuid instead of random string to make submission almost impossible to collide.

Hello Team KALE,
For your api-specifications documentation, would you be able to add to your documentation the HTTP status codes that your various responses generate? Having these status codes properly implemented and documented would allow for better error handling.

Example:

Thanks,
-Henry Agnew
LibreTexts Lead Full-Stack Developer
UC Davis, class of 2020

I wrote some setup docs a long time ago: https://github.com/lxylxy123456/ngshare#installation-and-setup
I just saw @rkevin-arch also wrote the documentation in https://github.com/lxylxy123456/ngshare/tree/master/testing#testing-setup
Maybe we should merge them?

A student may submit

"directory_tree": [{
    "path": "../../etc/passwd", 
    "content": "...",
}]

Should an assignment have some states? And the server can block submission when assignment is in an incorrect state? For example:

• not_released
• released
• deadline_passed
• feedback_uploaded

I'm not entirely sure if I should release a new version after PR #120. Releasing versions too frequently is kinda bad since users will have to keep updating, but also that feature is useful for at least a couple of people so we should probably make it available sooner rather than later. @lxylxy123456 What do you think?

@Lawrence37 Could you please specify a datetime format in the API spec? For example:
%Y-%m-%d %H:%M:%S %Z%z

Note: maybe this should not be used because it doesn't include information up to nanosecond

Great work so far. You have a cool project here.

I would recommend trying a few things to improve code maintainability down the road:

• set up a CI system for testing (CircleCI, Azure Pipelines, whatever)
• set up a code coverage tool for automated reporting
• add some documentation deployed on ReadTheDocs
• add badges for the above into the top of the main README
• consider adding the black project for python code consistency

Well done. cc/ @moorepants

https://github.com/lxylxy123456/ngshare/blob/master/api-specifications.md#request-3

@Lawrence37 Do we need to include all the file lists? ("notebooks":[...])

1. I think the assignment directory tree should be the same as submission directory tree (only files are different)
2. The file list will also be available if the instructor downloads it at /api/submission/<course_id>/<assignment_id>/<student_id>. If needed, we can add an option of listing the filenames without downloading them.

Could I please confirm if this is the correct default config.yaml?

  2   resources:
  3     limits:
  4       cpu: 100m
  5       memory: 128Mi
  6
  7 ngshare:
  8   # You may omit this, but the token will be randomly generated.
  9   # It's recommended to specify an API token here.
 10   hub_api_token: <secret string>
 11   admins: [myusername]
 12
 13 pvc:
 14   # Amount of storage for ngshare
 15   storage: 1Gi

I tried installing this to a separate namespace called ngshare and it keeps spiraling into a crash loop.

$ kubectl logs doltish-quoll-ngshare-8d78dd89b-c97ww -n ngshare
Traceback (most recent call last):
  File "/usr/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/usr/local/lib/python3.6/dist-packages/ngshare/__main__.py", line 3, in <module>
    main()
TypeError: main() missing 1 required positional argument: 'argv'

https://github.com/lxylxy123456/ngshare/blob/master/api-specifications.md#post-apisubmissioncourse_idassignment_idstudent_id

What does instructor+student mean? Can instructor submit for any student? Or instructor can submit as a student?

Make sure that administration can control who can create courses

Hi, Tom from Codecov here.

We noticed that you are using Codecov with fairly high frequency, and we’re so excited to see that! However, because you are not using our app, you may have experienced issues with uploading reports or viewing coverage information. This is due to rate-limiting issues from GitHub.

In order to prevent any future outages, we ask that you move over to our GitHub app integration.

The process is extremely simple and shouldn’t require more than a few clicks, and you should not expect any downtime. By moving to our app, you will no longer need an admin or separate account to manage the relationship with GitHub as the team bot.

Let me know if you have any questions, or if I can help at all with this process.

Have you tried uninstalling ngshare, and are there instructions for it? Is it as simple as helm delete ngshare --purge?
One thing I noticed was that ngshare is installed under the kube-system namespace. I don't know whether makes it difficult to uninstall. Might want to consider adding instructions with deploying ngshare on a different namespace like JupyterHub does, but this is up to you guys.

While reviewing the documentation, I made some changes which you will see in a pull request. There are some additional things that we may want to address.

• Building the HTML documentation results in this warning: html_static_path entry '_static' does not exist.
• Add all dependency installation instructions for rendering the tikz figure in the documentation. I get LaTeX error: "File 'standalone.cls' not found."
• Consistent capitalization for section titles. Ex: "Extra features" vs. "Extra Features"
• Prominently display a link to this repository in the contributor guide.
• Add developer installation instructions to the contributor guide.
• Debate placement of the "Install vngshare" section. I think it should go under the contributor guide.
• Consider removing the top level of the APIs hierarchy. In other words, make "API Definitions", "Authentication", etc. direct children of "APIS" rather than under "APIS" -> "APIs".
• Discuss whether or not "Removing" should be in the API definitions.
• Consider adding some formatting to "HTTP ### <reason>" texts in the "Request and Response Format" section.
• Talk about the 500 status code in the user documentation. Currently, it is only in the "Request and Response Format" section and it says to submit a bug report. A user may not see this message if it's not in the user documentation.
• Reword "Submissions of the instructor are not removed" in the course APIs section. It is not 100% clear what the sentence is saying.
• Debate the inclusion of the "Development History" section. It doesn't seem relevant to users and future developers.
• Add our justifications for using certain technologies in the "Decisions" section.
• Add more details to the "Development" section. For example, explain what the code does.
• Add introduction in the user documentation explaining what nghsare is. I missed the home page. I will review that.
• Explain that a plugin is needed for ngshare to work with nbgrader. This will probably be resolved when the "Install nghsare" section has been written, but I'm putting this here as a reminder.
• Explain what "JupyterHub authentication" is in the "Authentication" page.

I was wondering whether it would be better to use None instead of empty strings for the optional fields in ngshare. Currently nbgrader allows for the first name, last name and email of a student to be None.

but ngshare only allows for empty strings, not for None.

Not a big deal but wanted to see your thoughts.

We need to be able to remove assignments. This can serve as a temporary work-around for #13.

This problem happens after #18
Consider the API /api/assignments/<course_id>.
If we have one class ECS123, and a student not taking this class
Then for this user, /api/assignments/ECS123 returns "permission denied", and /api/assignments/ECS124 returns "course not found"

So any user who can access the service can list all courses using brute force.
Is this a problem?

When you create a new student the line student = self.find_user(student_id) looks for the student. When creating a student for the first time you get the error User not found.

https://github.com/lxylxy123456/ngshare/blob/master/api-specifications.md#get-apisubmissioncourse_idassignment_idstudent_id

Is the download going to download the latest submission? Can a instructor download an earlier version?

Port vserver to ngshare

The random string in nbgrader is used only to prevent students from accessing other students' submissions. The random string only appears in the inbound directory of the exchange. It is not used in the user's cache, so it does not serve as a unique submission identifier, at least for cached submissions. ngshare should be able to prevent unauthorized access by design, so the random string is not needed for ngshare.

This issue is for tracking the removal of random string. @aalmanza1998 and I will revise the API specifications first. @lxylxy123456 will ensure vserver and ngshare follow the new API.

We need to add a request to vserver to create a course.

Hi!

I was just testing out whether I could have ngshare run on the same PostgreSQL server as I have JupyterHub on. I'll document my issues here.

• The Docker image needs psycopg2. I used a custom image:

  FROM libretexts/ngshare:v0.5.1
  USER 0:0
  RUN pip install psycopg2-binary
  USER 65535:65535

• The database seems to be designed with Sqlite's dynamic-ish typing. Best case, ngshare should be tested against both sqlite and PostgreSQL. Example error, although I would expect many more due to Sqlite's lax behaviour wrt types:

  sqlalchemy.exc.ProgrammingError: (psycopg2.errors.DatatypeMismatch) foreign key constraint "assignment_files_assoc_table_left_id_fkey" cannot be implemented
  DETAIL:  Key columns "left_id" and "_id" are of incompatible types: text and integer.
  
  [SQL:
  CREATE TABLE assignment_files_assoc_table (
  	left_id TEXT NOT NULL,
  	right_id INTEGER NOT NULL,
  	PRIMARY KEY (left_id, right_id),
  	FOREIGN KEY(left_id) REFERENCES assignments (_id),
  	FOREIGN KEY(right_id) REFERENCES files (_id)
  )
  
  ]
  

• Sqlite creates the database automatically when opened, but psycopg2 as driver does not. I used a Kubernetes Job to create the database instead. This might need a simple code change somewhere.
• Unless I am mistaken, the PVC that gets allocated is only used for the Sqlite support, so that should be shut of when the installations opts for PostgreSQL.

Is PostgreSQL support something you folks would be interested in? I'd provide some patches if so.

Should GET /api/feedback/<course_id>/<assignment_id>/<student_id> return an error when there is no feedback (currently it returns an empty list)

If vserver is no longer used in any capacity to support the current code, it probably makes sense to move it to its own repo to avoid confusing users of ngshare.

Nice work folks!

The README for ngshare is very short. We need to add more information to it so that users and potential users understand what ngshare does.

This is a (noncomprehensive) to-do list. Please add items here by editing this post.

• What is ngshare?
• What can I use it for?
• Installation and setup

I get this error whenever I post an assignment and then try to list it. Any clue what might cause this?

Not sure if it's a good idea, but it would save us the trouble of packaging stuff manually. Now that chartpress is working with Travis, if we set up this that means Travis can handle everything.

Also, we might want to limit chartpress / pushing to PyPI to only run on the master branch when something is merged / pushed into it, rather than for all branches. IDK.

https://docs.travis-ci.com/user/deployment/pypi/

@lxylxy123456

I merged user-guide and master, and the new branch is now ecs193-final-report. If everything is fine we can then remove user-guide branch.

https://ngshare.readthedocs.io/en/ecs193-final-report/index.html

Add timestamp in the response for posting a submission.