cs_comments_service icon indicating copy to clipboard operation
cs_comments_service copied to clipboard

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

Open ghassanmas opened this issue 2 years ago • 1 comments

In order to resolve the following error:

tutor_nightly_local-forum-1                      | config.ru:18:in `block in <main>': undefined method `timeout=' for Rack::Timeout:Class (NoMethodError)
tutor_nightly_local-forum-1                      | 	from /app/cs_comments_service/vendor/bundle/ruby/3.0.0/gems/rack-1.6.13/lib/rack/builder.rb:55:in `instance_eval'
tutor_nightly_local-forum-1                      | 	from /app/cs_comments_service/vendor/bundle/ruby/3.0.0/gems/rack-1.6.13/lib/rack/builder.rb:55:in `initialize'

~~I have to change config.ru file following the documenation at https://github.com/zombocom/rack-timeout#sinatra-and-other-rack-apps~~ I couldn't change using the doc, I got an error execption, so I only way I was able to change it is to use ENV varaible.

ghassanmas avatar Oct 11 '22 10:10 ghassanmas

Thanks for the pull request, @ghassanmas! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

openedx-webhooks avatar Oct 11 '22 10:10 openedx-webhooks

@ghassanmas Thanks for catching and fixing this issue! @xitij2000 recently finalised his ruby3 PR -- can you rebase and address my comment above?

mtyaka avatar Nov 09 '22 08:11 mtyaka

@mtyaka the rebase is done, though I couldn't see your comments.

ghassanmas avatar Nov 09 '22 11:11 ghassanmas

Sorry about that @ghassanmas, I forgot to publish the comment, but I published it now. Thanks for rebasing!

mtyaka avatar Nov 09 '22 11:11 mtyaka

@mtyaka I resolved your comment and pushed, but I am still testing it... I had to push it so bceause the test enviroment can pick up that commit given its not same as my local... Anyway I will repot soon, the test will also be based with rebased i.e. the latest of @xitij2000 commits

ghassanmas avatar Nov 09 '22 12:11 ghassanmas

Tested all good!.

ghassanmas avatar Nov 09 '22 12:11 ghassanmas

Codecov Report

Base: 96.08% // Head: 96.06% // Decreases project coverage by -0.02% :warning:

Coverage data is based on head (62eb703) compared to base (712a8bd). Patch has no changes to coverable lines.

:exclamation: Current head 62eb703 differs from pull request most recent head eeb461e. Consider uploading reports for the commit eeb461e to get more accurate results

Additional details and impacted files
@@                Coverage Diff                @@
##           kshitij/ruby3     #395      +/-   ##
=================================================
- Coverage          96.08%   96.06%   -0.03%     
=================================================
  Files                 58       58              
  Lines               4522     4469      -53     
=================================================
- Hits                4345     4293      -52     
+ Misses               177      176       -1     
Impacted Files Coverage Δ
api/notifications_and_subscriptions.rb 87.50% <0.00%> (-1.39%) :arrow_down:
spec/support/matchers.rb 77.77% <0.00%> (-1.17%) :arrow_down:
app.rb 88.78% <0.00%> (-0.51%) :arrow_down:
models/concerns/searchable.rb 84.84% <0.00%> (-0.45%) :arrow_down:
api/comment_threads.rb 88.52% <0.00%> (-0.37%) :arrow_down:
models/subscription.rb 94.11% <0.00%> (-0.33%) :arrow_down:
models/comment.rb 87.60% <0.00%> (-0.30%) :arrow_down:
lib/task_helpers.rb 80.14% <0.00%> (-0.29%) :arrow_down:
api/comments.rb 92.42% <0.00%> (-0.23%) :arrow_down:
presenters/thread_utils.rb 97.22% <0.00%> (-0.22%) :arrow_down:
... and 13 more

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

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

codecov[bot] avatar Nov 10 '22 11:11 codecov[bot]

Tested all good!.

I've incorporated this commit into my ruby 3 upgrade PR since without that the forum service is broken on unicorn.

xitij2000 avatar Nov 16 '22 13:11 xitij2000

@ghassanmas Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks avatar Dec 05 '22 13:12 openedx-webhooks