CLI improvement before and during install #35011
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jolelievre
force-pushed
the
cli-improvement
branch
6 times, most recently
from
c7b4b7d
to
94d059c
Compare
…eating error in CLI command when the database access is not configured
…figuration is defined yet
…and warmup when no parameters are set
jolelievre
force-pushed
the
cli-improvement
branch
from
94d059c
to
64fc8a8
Compare
nicosomb
approved these changes
Jan 12, 2024
matthieu-rolland
approved these changes
Jan 12, 2024
ps-jarvis
added
the
Waiting for QA
matks
reviewed
Jan 12, 2024
| */ | ||
| class DatabaseConnection extends Connection | ||
| { | ||
| private const PARAMETERS_FILE = __DIR__ . '/../../../app/config/parameters.php'; |
There was a problem hiding this comment.
Suggestion: make it configurable with a construct
There was a problem hiding this comment.
I'm gonna try but I'm not sure it's easily doable I wanted to do it at first but it implies modifying the constructor compared to the parent, and changing this constructor might mess with Doctrine initialization so I can't guarantee it will be doable
| { | ||
| private const PARAMETERS_FILE = __DIR__ . '/../../../app/config/parameters.php'; | ||
|
|
||
| public function getDatabasePlatform() |
M0rgan01
approved these changes
Jan 12, 2024
boherm
approved these changes
Jan 15, 2024
AureRita
approved these changes
Jan 18, 2024
There was a problem hiding this comment.
Hi @jolelievre
Thank you for your PR, I tested it and it seems to works as you can see :
recording.64.webm
Link to the auto test : https://github.com/jolelievre/ga.tests.ui.pr/actions/runs/7475031416
Because the auto test is 🟢 and the PR seems to works as expected, It's QA ✔️
Thank you
AureRita
added
QA ✔️
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-
./bin/consolecan now be executed even when the shop is not installed yet, as well as./bin/console cache:clear- Add more logs in the install process especially when executing CLI install
-
LocalizationExtensionwas refactored to prevent some early fatal error when the shop was not installed,LocaleExtensionwas removed and merged into this extension- A wrapper class was added for DBAL connection, it prevents a bug when parameters are not configured, so the database platform cannot be guessed
- Yaml and twig linter were failing silently, they are now operational again
- To prevent any future regression a CLI was added to check that
./bin/consoleand./bin/console cache:clearcan be be executed without failure whether the shop is installed or notHTTP redirection from Tools method is disabled when running in CLI environment
-
SqlTranslationLoaderreturns an empty catalogue when the DB is not accessiblecomposer install, run./bin/consoleand./bin/console cache:clearpreviously it failed, now it succeeds even if some warnings are still left When you perform a CLI install you get more log output to track what is happening