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 Ruby 2.4 #1794

Merged
merged 5 commits into from
Jan 20, 2023
Merged

Upgrade to Ruby 2.4 #1794

merged 5 commits into from
Jan 20, 2023

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Nov 15, 2022

refs #1792

@ysbaddaden ysbaddaden self-assigned this Nov 15, 2022
@ysbaddaden ysbaddaden mentioned this pull request Nov 15, 2022
19 tasks
@ysbaddaden ysbaddaden marked this pull request as ready for review November 15, 2022 14:40
Base automatically changed from tech-debt/upgrade-to-ruby-2.3 to main November 25, 2022 17:29
@diegoliberman diegoliberman linked an issue Nov 28, 2022 that may be closed by this pull request
@diegoliberman
Copy link
Contributor

Let's wait until January to merge this, so that we send it to production when Julien is back from vacations.

@ysbaddaden
Copy link
Contributor Author

Hum, the integration tests on CI seem to be much slower all of a sudden in this branch (main is unaffected). I'm investigating to see what's happening.

@ysbaddaden
Copy link
Contributor Author

Selecting options is what's taking the most time in both Capybara & Cucumber specs. Each time it must select an option in a form, the browser hangs for a while, and I see the following message in the geckodriver log. It eventually continues but that inserts many hangs in the automated process that slows everything down (the rest feels normally fast).

Marionette      WARN    TimedPromise timed out after 500 ms

It may have something to do with the CdxSelect React component.

@ysbaddaden
Copy link
Contributor Author

Further reduced: the hangup occurs when we click the .Select-placeholder selector in CdxSelect#set (and other methods). Finding the element is quick, but either the click is slow or the action of displaying the options (.Select-option) is super slow.

@ysbaddaden
Copy link
Contributor Author

Found it: CdxSelect#select_elem is the source of the issue. We're searching for a parent element until .Select matches, which is taking over 2 seconds 😨

@ysbaddaden
Copy link
Contributor Author

I added an optimization to avoid blindly looking for the element when it's already the element we expect (it matches .Select). It fixed the slow issue that appeared with the upgrades (Ruby & dependencies).

There are still lots of cases where the fallback to the slow path is required (which I tried to optimize a bit). Reviewing the matchers in Page Objects to directly attach CdxSelect to .Select elements may speed up the integration tests further.

@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Jan 13, 2023

@matiasgarciaisaia I reworked the production Dockerfile to be based on ruby:2.4 directly instead of instedd/nginx-rails:2.4.

The changes for deployments are:

  • proxy target port is now 3000 (instead of 80)
  • add the RAILS_SERVE_STATIC_FILES=1 environment variable

This is already done in staging.

# Add config files
ADD docker/web-run /etc/service/web/run
EXPOSE 3000
CMD ["/app/docker/web-run"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The command actually starts whenever and puma processes. We might want to have a distinct container for whenever, like we do for sidekiq.

Alternative: use sidekiq-cron or sidekiq-scheduler instead.

@@ -1,4 +1,4 @@
FROM ruby:2.3
FROM ruby:2.4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we're moving to Debian Buster (oldstable) 💪

@ysbaddaden
Copy link
Contributor Author

BUG: the PDF is slightly different than on main. For example the CDx logo bleeds to other pages.

Generating the PDF relies on the abandoned and far outdated wkhtmltopdf binary through the wicked_pdf gem. A weird "webkit shrinking" mode is enabled by default and happens to behave differently depending on the wkhtmltopdf builds, despite using the exact same HTML source file.

After trying, unsuccessfully, to get a similar rendering, I chose to try out Prawn which proved as simple to use as ever and allowed to tweak the render to reach almost exactly the same render when zoomed at 400% (important because of dot matrix printer).

@ysbaddaden ysbaddaden merged commit 1a6ddb2 into main Jan 20, 2023
@ysbaddaden ysbaddaden deleted the tech-debt/upgrade-to-ruby-2.4 branch January 20, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Ruby 2.4 Improve performance of the print PDF actions
2 participants