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

fixing issue #466: On assignments allow the instructor to set a day/time for it to be make visible #502

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

kwizeras
Copy link

So far, in trying to solve this issue, I have made changes in four files: assignment.jsx, crud.py, assignSlice.js, student.py.
My plan was to create to replace the visibility switch by two new columns for the instructor to set the visibility date and the hiding date at creation of each assignment.

Right now I am able to create new assignments with visible and hiding dates only for options "Search for Exercises" and "Write an Exercise". However, for options "Choose Readings" and "Choose Exercises," I failed to fetch the specified assignments, and thus, an empty assignment was created.

Another issue that I am seeing is that, even though I am logged in as an instructor, when I navigate to >User>Instructors Page, I get a blank page and an error saying react-router-dom.js?v=70ae4b27:215 No routes matched location "/runestone/admin/admin"

@kwizeras kwizeras requested a review from bnmnetp as a code owner July 10, 2024 20:24
@bnmnetp
Copy link
Member

bnmnetp commented Jul 10, 2024

OK, you should be keeping your fork up to date with my latest changes, I think that may help with some things.

But the bigger issue is that you have added two new attributes to an assignment. That is good. But when the Javascript triggers the new_assignment call that makes an API call to new_assignment on the server. The function on the server (see instructor.py new_assignment) has request_data that is defined to be AssignmentIncoming The server automatically validates that the data that is coming from the client matches the definition of AssignmentIncoming as defined in schemas.py. If it does not match then you get the 422 Unprocessable error. In this case you have not updated either the model.py for the schemas.py files to add the two new columns for the database.

The menu options will not work when you are using the server on localhost:5173
This is not meant to be fully functioning only for quick development turnaround for doing React development.

You should have a separate tab open to localhost/runestone/admin/admin

Tagging @pearcej and @jjjonesj

@kwizeras
Copy link
Author

Thanks for the comments!
I have updated my updated my fork and added the new column under schemas.py and models.py and adjusted assignSlice.js accordingly. These steps solved some of the errors I was getting. Now I can create a new assignments with duedate, visibledate, and hidingdate set at creation.

However, I am still getting an internal error when I navigate to localhost/runestone/admin/admin to open the Instructor's Page. what might causing the error?

@bnmnetp
Copy link
Member

bnmnetp commented Jul 11, 2024

If you are getting an internal error then you should be able to find a ticket for that in the tickets folder. You can edit it even though it might complain and say it is a binary file. That will get us further along. You can also probably see the problem by looking at the log file for runestone.

@bnmnetp
Copy link
Member

bnmnetp commented Jul 11, 2024

If you have fixed it then you should make sure to push, so I can see the changes here.

@jjjonesj
Copy link

How do we navigate to the tickets folder for the ticket or find the runestone log file?

@bnmnetp
Copy link
Member

bnmnetp commented Jul 11, 2024

The documentation covers both of those things.

Please look at https://runestone-monorepo.readthedocs.io/en/latest/debugging.html

Bookmark the documentation, it is searchable, and there to help you so you don't have to ask me.

@jjjonesj
Copy link

I led @kwizeras through the documentation, showed him where the ticket files were, and had him pull up the log files from the command:

docker compose logs --tail 100

I saw several errors regarding a failure of matching files or to fetch the user even though he is logged in. I also walked him through the ticket process from the internal error from the localhost page and we're still looking for the ticket from today. I know you said that it could take some time for that folder to update so, we are looking at a previous one that may have a similar error. He told me to ask, what are we supposed to edit when we get there? I looked through your documentation as you instructed and attempted the ipython deal but, he said he was not sure we should do that and stopped. I would really appreciate some civil guidance. I have some of the errors that came up attached. I'm not sure if this is what you are looking for according to your documentation and previous comments. If not, please let me know and I will try to work this out with him further. Thank you.

Screenshot 2024-07-12 095701
Screenshot 2024-07-12 095833
Screenshot 2024-07-12 095905
Screenshot 2024-07-12 100943

@bnmnetp
Copy link
Member

bnmnetp commented Jul 12, 2024

  1. If the system says he's not logged in then he's not logged in. I can't tell without actually watching what might be wrong, but it never hurts to logout and login again.
  2. The file matching is nothing to worry about. The nginx server has to remap a lot of really old URLs to provide backward compatibility for older books etc.
  3. At the end of the last screenshot is the critical point. It is telling you that the column kind is missing from the assignments table. Yet git tells me that I added that column 4 months ago. which means that the database is not in sync with the model. If you are using a recent version of build.py it would have told you that towards the end of the build process. I would suggest that you run the following commands to try to get back up to date.
docker compose down db
docker compose stop
docker compose up -d db
docker compose run rsmanage rsmanage initdb
docker compose up -d
alembic stamp head

The docker compose down command removes the container, so you can start from a fresh database. Now you will have to rebuild whatever book you are using to work on as well as recreate usernames and getting set up as an instructor.

alembic stamp head will record the revision of the database as current. When you add the new columns you will need to make an alembic revision so that other developers can update their database.

The suggestion to use ipython is to make it easy for you to interactively test your changes to any of the functions in crud.py. You absolutely should do that as it is much easier than rebuilding containers or finding errors in the logs.

@kwizeras
Copy link
Author

We did all the updates as suggested, but we kept receiving the internal error from before. When I checked my build.log, there was a message saying that my current Python version (3.10.12) is not allowed by the project (see image)
image .

I run
sudo apt update sudo apt install python3.11 python3.11-venv python3.11-dev

to update my python to Python3.11 (rs-py3.11) creativecoder@STU-CFY63B3: After doing all that we fixed the internal error issue and were able to navigate to the instructor page using the old interface, but we were not able to do the same on the new interface. It always said that I am not not logged in. Aren't these two interfaces supposed to be sharing data?

build.log.txt
The attached buil.log file highlights some things that might be causing the issue, but we're not exactly sure how to approach them.

@jjjonesj
Copy link

I wanted to follow up on what @kwizeras said yesterday with the changes that we've made. We changed the hidden and visible objects in the instructor file back into booleans. We also made a lot of changes to the fetch assignments function in the crud file. We are currently unable to test it as our databases are not working. We've been using @kwizeras's to test due to mine having issues for a long time and my space to code. We don't have a lot of time left as tomorrow is my last day. I'd be very grateful if you could help us test or fix my partner's database because we're not sure what caused the problem over the weekend. Thank you

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

Can you be more specific about what it means that @kwizeras database is broken? I was also not aware that you were having database issues for a long time.

These should be reliably cleaned up by the steps I gave you above. Unless you encountered some kind of error while running those commands.

@jjjonesj
Copy link

jjjonesj commented Jul 16, 2024

My database would work for a while, allowing me to see all the pages on the old interface, including the instructor page. Then it would stop and give me ticket/internal errors. I would reset it and it would work again, then stop. I've had this going on since before working on this issue and so, we decided it would be best to just use my partner's. From his post and what I've seen yesterday, he's having an issue with his python version stopping him from building. He followed your instructions on Friday and didn't seem to have a problem. He also went back over the documentation instructions and is still having errors. I will have him elaborate more if needed but, I think he summed up his main issue in the previous post to mine.

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

Unfortunately you hit the same problem as I did yesterday morning in the build.log file you attached. It took me most of the day to fix the problem. I pushed those fixes to the main repo late yesterday.

@kwizeras
Copy link
Author

Unfortunately you hit the same problem as I did yesterday morning in the build.log file you attached. It took me most of the day to fix the problem. I pushed those fixes to the main repo late yesterday.

Does this mean that updating my fork with your yesterday's push would resolve my issue?

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

yes, it should.

@kwizeras
Copy link
Author

yes, it should.

I got all the updates, but I am still getting errors. And part of this might be the Python version I am usng. When run the run the Python build command using Python3.10, book fail to build and the build.log ask me to update the python version. But when i do update it the build runs fine (see image:,
image
but i get several errors when I run docker compose up and the local host gives a blank page with this message {"detail":{}} . @bnmnetp what python version are you using on your end? How can approach this?

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

What server errors?? You need to at least get me some log messages indicating the errors you are getting. There is no way I can just guess what might have gone wrong.

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

I have both python 3.11 and 3.10. I fixed the issue with the book server so you can use either.

@jjjonesj
Copy link

While @kwizeras is compiling his error logs, we solved the merge conflicts from your additions to our changes. I want to say thank you very much for those! Were there any other files that needed to be updated in relation to this project? I know the student and instructor files use the fetch assignment function with the is_visible boolean but, wasn't sure if that actually needed to be modified in any way.

@kwizeras
Copy link
Author

kwizeras commented Jul 16, 2024

What server errors?? You need to at least get me some log messages indicating the errors you are getting. There is no way I can just guess what might have gone wrong.

When I run (rs-py3.10) creativecoder@STU-CFY63B3:~/rs$ poetry run python ./build.py using Python3.10 book fail to build.
image

Build log for book server:
image

Build log for assignment server:
image

Build log for interactives:
interactivebuild.log.txt

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

@kwizeras I think you are missing some of my latest updates. Look in the pyproject.toml file in the projects/book_server folder. You should see a couple of lines:

[tool.poetry.dependencies]
python = "^3.10"

If it says python="^3.11" then it is out of date.

You said you were getting server errors. The build logs don't help with server errors. You need to use the docker compose logs xxxx command where xxx is the name of the server giving you the error.

1 similar comment
@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

@kwizeras I think you are missing some of my latest updates. Look in the pyproject.toml file in the projects/book_server folder. You should see a couple of lines:

[tool.poetry.dependencies]
python = "^3.10"

If it says python="^3.11" then it is out of date.

You said you were getting server errors. The build logs don't help with server errors. You need to use the docker compose logs xxxx command where xxx is the name of the server giving you the error.

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

@jjjonesj sorry about the merge conflict. That was related to some other work I am doing for Peer Instruction. To accomodate everything involved the vclause is going to need to look something like:

now = datetime.datetime.utcnow()
if is_visible:
    vclause = or_(Assignment.visible == is_visible,
                            and_(Assignment.visibleon >= now,
                                      Assignment.hiddenon <= now)
                             )

That allows for the backward compatibility case where we don't have dates as well as the new case where we have a dates.

@jjjonesj
Copy link

jjjonesj commented Jul 16, 2024

No problem, I added in what you just suggested with your new additions. Does this work better and go along with my previous question posted before @kwizeras's?

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

@jjjonesj
That is better but you cannot just delete the else part of the conditional for the vclause otherwise you will get an undefined variable if it does not get set. Setting it to True essentially means we don't care what the value is, so in the context of "anding" together all the clauses True will not cause the and_ function to become False.

…e else statement back. Mistakenly deleted it.
@jjjonesj
Copy link

Sorry, that was an accident. We had deleted everything we put there previously and that got caught in there. Was there anything else?

@kwizeras
Copy link
Author

kwizeras commented Jul 16, 2024

@kwizeras I think you are missing some of my latest updates. Look in the pyproject.toml file in the projects/book_server folder. You should see a couple of lines:

[tool.poetry.dependencies]
python = "^3.10"

If it says python="^3.11" then it is out of date.

You said you were getting server errors. The build logs don't help with server errors. You need to use the docker compose logs xxxx command where xxx is the name of the server giving you the error.

I got the updates containing the two new lines you pointed out:
[tool.poetry.dependencies]
python = "^3.10"

But, here are the server errors that I am getting:
docker compose logs book gives me:
bookservererror.txt

docker compose logs runestone gives me:
runestoneServerError .txt

docker compose logs nginx gives me :
nginxServiceError.txt

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

OK, you need to read them, not just ask me. I know you know how to deal with this:

  45   │ book-1  |   File "/usr/local/lib/python3.11/site-packages/rsptx/db/crud.py", line 1275, in fetch_assignments
  46   │ book-1  |     now = datetime.utcnow()
  47   │ book-1  |           ^^^^^^^^^^^^^^^
  48   │ book-1  | AttributeError: module 'datetime' has no attribute 'utcnow'

That was a typo by someone (maybe me) because it should be datetime.datetime.utcnow() It is just a plain python error that you could look up.

@kwizeras
Copy link
Author

OK, you need to read them, not just ask me. I know you know how to deal with this:

  45   │ book-1  |   File "/usr/local/lib/python3.11/site-packages/rsptx/db/crud.py", line 1275, in fetch_assignments
  46   │ book-1  |     now = datetime.utcnow()
  47   │ book-1  |           ^^^^^^^^^^^^^^^
  48   │ book-1  | AttributeError: module 'datetime' has no attribute 'utcnow'

That was a typo by someone (maybe me) because it should be datetime.datetime.utcnow() It is just a plain python error that you could look up.

Thanks,
I am going through them one by one.

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

The runestone server error is because you have eliminated the .visible column from the model which you can not do. You have to keep it for backward compatibility.

@bnmnetp
Copy link
Member

bnmnetp commented Jul 16, 2024

Just to drive home the point I was trying to make a couple days ago. The error in the book server could have been caught and fixed very easily by testing it from the ipython shell as is demonstrated in the documentation.

@jjjonesj
Copy link

jjjonesj commented Jul 17, 2024

Hi, I added the .visible column back to the models.py file and it appears @kwizeras is still having problems with the database. He's currently compiling all of those errors for you. However, since it is my last day, I wanted to know what else there might be to update with this issue in the files?

@kwizeras
Copy link
Author

Hi, I added the .visible column back to the models.py file and it appears @kwizeras is still having problems with the database. He's currently compiling all of those errors for you. However, since it is my last day, I wanted to know what else there might be to update with this issue in the files?

OK, you need to read them, not just ask me. I know you know how to deal with this:

  45   │ book-1  |   File "/usr/local/lib/python3.11/site-packages/rsptx/db/crud.py", line 1275, in fetch_assignments
  46   │ book-1  |     now = datetime.utcnow()
  47   │ book-1  |           ^^^^^^^^^^^^^^^
  48   │ book-1  | AttributeError: module 'datetime' has no attribute 'utcnow'

That was a typo by someone (maybe me) because it should be datetime.datetime.utcnow() It is just a plain python error that you could look up.

OK, you need to read them, not just ask me. I know you know how to deal with this:

  45   │ book-1  |   File "/usr/local/lib/python3.11/site-packages/rsptx/db/crud.py", line 1275, in fetch_assignments
  46   │ book-1  |     now = datetime.utcnow()
  47   │ book-1  |           ^^^^^^^^^^^^^^^
  48   │ book-1  | AttributeError: module 'datetime' has no attribute 'utcnow'

That was a typo by someone (maybe me) because it should be datetime.datetime.utcnow() It is just a plain python error that you could look up.

I have fixed those typo errors from both crud.py and student.py. But the whole server error was not resolved. I also realized that most of the error that involved files that we did modify as part of this project/issue we are trying to fix.

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

Successfully merging this pull request may close these issues.

3 participants