chore: upgrade to ruby 3 #392

xitij2000 · 2022-09-21T06:15:09Z

This PR adds support for running cs_comments_service on Ruby 3 in addition to Ruby 2.5

Given that Ruby 2.5 is EOL, 2.7 will be EOL within months. Ruby 3 gives us a bit over a year to upgrade to Ruby 3.1.

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: "None"

Testing instructions:

This PR can be used with the corresponding devstack PR to manually test this PR.

Testing should involve manual interaction with all the aspects of the forum.

Author notes and concerns:

The code has been tested and is working with 3.1 as well, but there are issues when running 3.1 on CI. It will hopefully not require much effort to upgrade to 3.1

openedx-webhooks · 2022-09-21T06:15:12Z

Thanks for the pull request, @xitij2000!

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

codecov · 2022-09-21T06:18:42Z

Codecov Report

❗ No coverage uploaded for pull request base (master@063fd26). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 712a8bd differs from pull request most recent head afeb30c. Consider uploading reports for the commit afeb30c to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #392   +/-   ##
=========================================
  Coverage          ?   96.08%           
=========================================
  Files             ?       58           
  Lines             ?     4522           
  Branches          ?        0           
=========================================
  Hits              ?     4345           
  Misses            ?      177           
  Partials          ?        0

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

natabene · 2022-09-21T15:22:46Z

@xitij2000 Thank you for the contribution, let me know once this is ready.

ghassanmas · 2022-10-24T16:17:10Z

@xitij2000 Following our today BTR call, we were disucssing whether discussion (backend service) should run on Ruby 3 or not.

One factor obviously is deadline of this PR, can you by any chance estimae when this would be ready/merged?

Also can you confirm that the alternative is to run the service on a ruby version (2.5.7) ref which already has reached its EOL?

Note: I personally tested this locally but I had to do #395 first and the test was run tutor based using the forum plugin, overhangio/tutor-forum#11 and we are in the process of testing in the acutal production.

jmbowman · 2022-10-24T16:55:19Z

It looks like there's also a Ruby 2.7.6 upgrade option that might be easier, but that would also go EOL about 3 months before Palm is released. I think this one slipped under the radar because our Ruby entries in the support windows doc were outdated.

xitij2000 · 2022-10-26T11:23:14Z

@xitij2000 Following our today BTR call, we were disucssing whether discussion (backend service) should run on Ruby 3 or not.

One factor obviously is deadline of this PR, can you by any chance estimae when this would be ready/merged?

Also can you confirm that the alternative is to run the service on a ruby version (2.5.7) ref which already has reached its EOL?

Note: I personally tested this locally but I had to do #395 first and the test was run tutor based using the forum plugin, overhangio/tutor-forum#11 and we are in the process of testing in the acutal production.

I will try to prioritise this in the upcoming sprint. Initially the aim was just to get tests running on Ruby 3.0 to catch any new failures, and slowly fix things over time, but I can put some of my CC time to getting this running.

@jmbowman I aimed for 3.x for that very reason, that 2.7 was also close to EOL. I will see if I can get all tests passing and if the forum is running without issues on 3.x, and if not I'll see if 2.7 is easier.

mtyaka · 2022-11-09T08:47:11Z

@xitij2000 This looks great and it required less changes than I thought it would!
I think we'll want to pull in the change from #395 before merging, but other than that everything looks good and seems to be working well (I tested it on The Olive demo instance) which is already running code from this branch on ruby 3).

xitij2000 · 2022-11-09T09:10:02Z

presenters/thread.rb

@@ -96,7 +96,7 @@ def get_paged_merged_responses(thread_id, responses, skip, limit, recursive=fals
    paged_response_ids = limit.nil? ? response_ids.drop(skip) : response_ids.drop(skip).take(limit)
    if recursive
      content = Comment.where(comment_thread_id: thread_id).
-        or({:parent_id => {"$in" => paged_response_ids}}, {:id => {"$in" => paged_response_ids}}).
+        any_of({:parent_id => {"$in" => paged_response_ids}}, {:id => {"$in" => paged_response_ids}}).


The way this API works has changed in mongoid.

So while the previous version was equivalent to:
comment_thread_id == thread_id AND (parent_id in paged_response_ids OR id in paged_response_ids)
It is now equivalent to
comment_thread_id == thread_id OR parent_id in paged_response_ids OR id in paged_response_ids)
since the where is merged into the or. Using any_of fixes this issue (ref)

xitij2000 · 2022-11-09T09:10:38Z

spec/api/comment_spec.rb

@@ -331,7 +331,7 @@ def test_unicode_data(text)
      page_3 = parse last_response.body


-      all_items["collection"].should == (
+      expect(all_items["collection"]).to eq(
        page_1["collection"] +
        page_2["collection"] +


These just update to the newer syntax.

xitij2000 · 2022-11-09T09:11:46Z

spec/factories.rb

@@ -40,6 +40,6 @@
    body { Faker::Lorem.paragraph }
    course_id { comment_thread.course_id }
    commentable_id { comment_thread.commentable_id }
-    endorsed false
+    endorsed { false }
  end
 end


Updates tot accommodate new versions of Faker, and migration from FactoryGirl -> FactoryBot

xitij2000 · 2022-11-09T09:14:50Z

app.rb

+  def t(*args, **kwargs)
+    I18n.t(*args, **kwargs)


Ruby generally uses more keyword arguments, which have had major changes in Ruby 3. Without this, keyword arguments are no longer passed and translations break when paramaterised translations are used (in one place).

xitij2000 · 2022-11-09T11:16:20Z

I've got all the tests running and passing on both Ruby 3.0 and 3.1, however with Ruby 3.1 the CI seems to fail due to failed dependencies when building an extension. I might not have time to resolve that, but it is working otherwise.

mtyaka · 2022-11-14T14:02:43Z

Thanks a lot for taking care of this @xitij2000! 👍

I tested this: I verified that the forums are working correctly on the Olive demo instance.
I read through the code
I checked for accessibility issues (N/A)
Includes documentation (N/A)

arbrandes · 2022-12-05T15:10:09Z

@asadazam93, is this something you could look at in the next couple of days? We want to have it in as part of the Olive release, since Ruby < 3 is out of support (and has been for a while).

asadazam93 · 2022-12-05T15:54:38Z

@xitij2000 Is this a breaking change?

xitij2000 · 2022-12-06T12:39:02Z

@xitij2000 Is this a breaking change?

It should not be. This PR is tested against both the current version of Ruby (2.5.7) and on Ruby 3.0.x. However, the idea is to eventually start using Ruby 3.0, so it sets the stage for that.

ghassanmas · 2022-12-08T10:35:22Z

Hi folks, Would it be possible for this to be merged today or by tomorrow at least. Since then it need to be backported to olive and then tested again with the recent commits to olive...etc. Thus the sooner this is merged the less urguent things would need to be done for the olive relaese.

I appercaite your understaning and happy to help if anything is needed with this.

Upgrade the service to work on both on Ruby 2.5.7 (currently used by the repo) and 3.0 (which is currently supported). Additionally, switch from running tests inside a docker container launched through scripts to directly setting up and testing inside GitHub's containers.

Also update readmde about how to update the rack-timeout

arbrandes

This has been tested in production via the Tutor demo/testing deployment, and no issues were found. Approved!

openedx-webhooks · 2022-12-09T11:05:14Z

@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Upgrade the service to work on both on Ruby 2.5.7 (currently used by the repo) and 3.0 (which is currently supported). Additionally, switch from running tests inside a docker container launched through scripts to directly setting up and testing inside GitHub's containers. Co-authored-by: Ghassan Maslamani <[email protected]>

openedx-webhooks added core committer open-source-contribution PR author is not from Axim or 2U labels Sep 21, 2022

xitij2000 force-pushed the kshitij/ruby3 branch from e44db28 to 0830b1f Compare September 21, 2022 06:16

xitij2000 force-pushed the kshitij/ruby3 branch from 0830b1f to 75afe61 Compare September 21, 2022 06:34

xitij2000 changed the title ~~kshitij/ruby3~~ chore: upgrade to ruby 3 Sep 21, 2022

xitij2000 force-pushed the kshitij/ruby3 branch from 75afe61 to 207d17c Compare September 21, 2022 08:25

ghassanmas mentioned this pull request Oct 11, 2022

feat: upgrade to olive overhangio/tutor-forum#11

Merged

ghassanmas mentioned this pull request Oct 24, 2022

PR(s) Needs to be backported or created to Olive openedx/wg-build-test-release#205

Closed

ghassanmas mentioned this pull request Oct 24, 2022

feat: add forum with ruby 3 overhangio/openedx-release-demo#20

Merged

jmbowman mentioned this pull request Oct 24, 2022

Periodic support windows doc updates edx/edx-arch-experiments#85

Closed

xitij2000 force-pushed the kshitij/ruby3 branch 4 times, most recently from 0fab65e to 41946be Compare November 3, 2022 11:58

mtyaka mentioned this pull request Nov 9, 2022

chore: use correct synatx for rack-timeout for ruby 3 #395

Closed

xitij2000 commented Nov 9, 2022

View reviewed changes

xitij2000 marked this pull request as ready for review November 9, 2022 09:19

xitij2000 force-pushed the kshitij/ruby3 branch from 53684c1 to 712a8bd Compare November 9, 2022 11:11

xitij2000 force-pushed the kshitij/ruby3 branch from 712a8bd to 0cf3536 Compare November 14, 2022 06:27

mtyaka approved these changes Nov 14, 2022

View reviewed changes

xitij2000 force-pushed the kshitij/ruby3 branch from c713baa to 30220dc Compare November 16, 2022 13:09

ghassanmas mentioned this pull request Dec 1, 2022

feat: support mongodb+srv urls overhangio/tutor-forum#10

Merged

arbrandes requested a review from asadazam93 December 5, 2022 15:09

arbrandes mentioned this pull request Dec 6, 2022

Ruby 3 upgrade openedx/wg-build-test-release#237

Closed

1 task

ghassanmas mentioned this pull request Dec 7, 2022

Open release/olive.master #400

Closed

ghassanmas mentioned this pull request Dec 9, 2022

Kshitij/ruby3 #401

Closed

xitij2000 and others added 2 commits December 9, 2022 16:03

chore: use correct synatx for rack-timeout for ruby 3

afeb30c

Also update readmde about how to update the rack-timeout

xitij2000 force-pushed the kshitij/ruby3 branch from 30220dc to afeb30c Compare December 9, 2022 10:34

arbrandes approved these changes Dec 9, 2022

View reviewed changes

xitij2000 mentioned this pull request Dec 9, 2022

feat: Update testing system #391

Closed

xitij2000 merged commit 3420c0a into master Dec 9, 2022

keithgg mentioned this pull request Dec 12, 2022

feat: operators can prefix elasticsearch indexes for multi-tenancy #404

Merged

xitij2000 deleted the kshitij/ruby3 branch July 14, 2023 08:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade to ruby 3 #392

chore: upgrade to ruby 3 #392

xitij2000 commented Sep 21, 2022 •

edited

Loading

openedx-webhooks commented Sep 21, 2022 •

edited

Loading

codecov bot commented Sep 21, 2022 •

edited

Loading

natabene commented Sep 21, 2022

ghassanmas commented Oct 24, 2022

jmbowman commented Oct 24, 2022

xitij2000 commented Oct 26, 2022

mtyaka commented Nov 9, 2022 •

edited

Loading

xitij2000 Nov 9, 2022

xitij2000 Nov 9, 2022

xitij2000 Nov 9, 2022

xitij2000 Nov 9, 2022

xitij2000 commented Nov 9, 2022

mtyaka commented Nov 14, 2022

arbrandes commented Dec 5, 2022

asadazam93 commented Dec 5, 2022

xitij2000 commented Dec 6, 2022

ghassanmas commented Dec 8, 2022

arbrandes left a comment

openedx-webhooks commented Dec 9, 2022

chore: upgrade to ruby 3 #392

chore: upgrade to ruby 3 #392

Conversation

xitij2000 commented Sep 21, 2022 • edited Loading

openedx-webhooks commented Sep 21, 2022 • edited Loading

codecov bot commented Sep 21, 2022 • edited Loading

Codecov Report

natabene commented Sep 21, 2022

ghassanmas commented Oct 24, 2022

jmbowman commented Oct 24, 2022

xitij2000 commented Oct 26, 2022

mtyaka commented Nov 9, 2022 • edited Loading

xitij2000 Nov 9, 2022

Choose a reason for hiding this comment

xitij2000 Nov 9, 2022

Choose a reason for hiding this comment

xitij2000 Nov 9, 2022

Choose a reason for hiding this comment

xitij2000 Nov 9, 2022

Choose a reason for hiding this comment

xitij2000 commented Nov 9, 2022

mtyaka commented Nov 14, 2022

arbrandes commented Dec 5, 2022

asadazam93 commented Dec 5, 2022

xitij2000 commented Dec 6, 2022

ghassanmas commented Dec 8, 2022

arbrandes left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Dec 9, 2022

xitij2000 commented Sep 21, 2022 •

edited

Loading

openedx-webhooks commented Sep 21, 2022 •

edited

Loading

codecov bot commented Sep 21, 2022 •

edited

Loading

mtyaka commented Nov 9, 2022 •

edited

Loading