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

Upgrade to Ubuntu 22.04 and Ruby3 #6672

Merged
merged 12 commits into from
Aug 2, 2023

Conversation

pretendWhale
Copy link
Contributor

@pretendWhale pretendWhale commented Jul 20, 2023

Motivation and Context

Upgrade base docker image to use ubuntu 22.04 and upgrade ruby from 2.7 to 3.0

Your Changes

Description:
Upgraded to ubuntu 22.04 and ruby 3. This requires a few additional changes. In particular:

  • QR code scanning has been moved to a python function, because the gem we were using was not maintained and I wasn't able to find a suitable replacement. This also requires installing python3-dev
  • postgres updated to version 14, the default on ubuntu 22.04
  • SortedSet is removed in ruby3, so instances of SortedSet are replaced with Set (and sorted using the builtin sort method)
  • modified tests for login validation to use a script with custom return values rather than mocking the return
  • Various other minor changes and test updates

Type of change (select all that apply):

  • Breaking change (fix or feature that would cause existing functionality to change)

Testing

Ran the test suite to ensure all tests pass, and tested functionality in the web interface

Questions and Comments (if applicable)

Checklist

  • I have performed a self-review of my own code.
  • 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 on Coveralls.
  • I have updated tests for my changes.
  • I have made changes to the documentation and linked to that pull request below.

Pull request to make documentation changes (if applicable)

@coveralls
Copy link
Collaborator

coveralls commented Jul 21, 2023

Pull Request Test Coverage Report for Build 5732108720

  • 25 of 28 (89.29%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 91.817%

Changes Missing Coverage Covered Lines Changed/Added Lines %
app/models/exam_template.rb 0 3 0.0%
Totals Coverage Status
Change from base Build 5695525132: 0.001%
Covered Lines: 37733
Relevant Lines: 40556

💛 - Coveralls

config/dummy_invalidate.sh Outdated Show resolved Hide resolved
app/views/annotation_categories/_annotation_text.html.erb Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
db/migrate/20121028211448_testing.rb Outdated Show resolved Hide resolved
app/jobs/split_pdf_job.rb Show resolved Hide resolved
app/jobs/split_pdf_job.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@pretendWhale great thanks for the update.

  1. I was able to run the migrations without error. Feel free to put in the error message you were seeing (if it's still happening).
  2. I rolled back several unnecessary package updates. As far as I could tell the only one that was required for Ruby 3.0 was rqrcode (which I kept).
  3. When I seeded the database I did get an error related to the scanned exams:
NoMethodError: undefined method `quality=' for #<ExamTemplate id: 1, filename: "midterm1-v2-test.pdf", num_pages: 6, created_at: "2023-08-01 15:15:18.280322999 -0400", updated_at: "2023-08-01 15:15:18.280322999 -0400", name: "midterm1-v2-test", cover_fields: "", automatic_parsing: false, crop_x: nil, crop_y: nil, crop_width: nil, crop_height: nil, assessment_id: 10>
/app/app/models/exam_template.rb:246:in `block in save_cover'
/app/app/models/exam_template.rb:245:in `from_blob'
/app/app/models/exam_template.rb:245:in `save_cover'
/app/app/models/exam_template.rb:47:in `create_with_file'
/app/lib/tasks/scanned_exam.rake:26:in `block (2 levels) in <main>'
/app/db/seeds.rb:24:in `<main>'

Did you encounter this (and is that what you were hoping to fix with your changes to split_pdf_job.rb)?

@pretendWhale
Copy link
Contributor Author

@david-yz-liu thanks for the review!

  1. The migrations ran without errors, but there were test errors due to extraneous db columns. That said, it looks like at some point errors were introduced into my structure.sql, which was causing the issue.
  2. Fair enough, that makes sense.
  3. I encountered issues with the splitting pdfs, and so yes that's what my change in split_pdf_job.rb is me. I missed the seeding issue (and it didn't come up in the tests) - I made an equivalent change to exam_template.rb which I'll push

@david-yz-liu david-yz-liu merged commit 13d47c5 into MarkUsProject:master Aug 2, 2023
2 checks passed
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