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

Fix database not available (Zeitwerk and Docker error) #103

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

armandfardeau
Copy link
Contributor

@armandfardeau armandfardeau commented Jun 13, 2023

This modules use a database for initialization but in some cases (Zeitwerk checks or precompilation of assets on Docker) a database is not available.

This P.R. allows usage in theses cases.

@armandfardeau armandfardeau changed the base branch from master to develop June 16, 2023 08:43
@armandfardeau armandfardeau force-pushed the fix/precompile-on-docker branch from 508805d to 228a491 Compare June 19, 2023 08:49
@armandfardeau armandfardeau force-pushed the fix/precompile-on-docker branch from 228a491 to 6a610dd Compare August 31, 2023 09:50
@armandfardeau armandfardeau changed the title Fix precompile on docker when no database is available Fix database not available (Zeitwerk and Docker error) Aug 31, 2023
@armandfardeau armandfardeau force-pushed the fix/precompile-on-docker branch from 6a610dd to 770b390 Compare August 31, 2023 10:12
@codecov
Copy link

codecov bot commented Aug 31, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (abbf0c6) 96.79% compared to head (5c2b648) 96.79%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #103   +/-   ##
========================================
  Coverage    96.79%   96.79%           
========================================
  Files           58       58           
  Lines         1153     1153           
========================================
  Hits          1116     1116           
  Misses          37       37           
Files Changed Coverage Δ
lib/decidim/term_customizer/i18n_backend.rb 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@sinaeftekhar sinaeftekhar left a comment

Choose a reason for hiding this comment

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

Dear @armandfardeau
Many thanks for the PR. There are couple of typos, which I made a comment on those.

@@ -78,6 +78,22 @@
end
end

context "when the translation query raises ActiveRecord::StatementInvalid" do
it "returns and empty result" do
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you mean an, not and

end

context "when there is no database connection" do
it "returns and empty result" do
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you mean an, not and

@Quentinchampenois
Copy link

Hello @sinaeftekhar,
Thank you for the review, I think I will continue this PR

If the change request only concerns typos in tests I can open a new PR to fix typos because I don't have write rights on the @armandfardeau's fork

@Quentinchampenois
Copy link

Hello, this PR won't be continued here but I can apply change request on a new one if needed

FYI, we already use this fix on our fork for containerized apps

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