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

IBX-3740: Prepended default Core settings #182

Merged
merged 5 commits into from
Mar 27, 2024
Merged

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Dec 21, 2022

Question Answer
JIRA issue IBX-3740
Type bug
Target Ibexa version v4.5+
BC breaks no

This PR foremost moves loading default settings into prepend stage. My initial idea was to move everything into load stage, however processing merged project configuration for ibexa extension is occurs before IbexaCoreExtension::load stage, therefore some of settings created by configureGenericSetup need to be available before load as well (e.g. purge_type parameter value).

Some side issues were fixed as well:

  • dropped setting parameter for dropped ezdesign PHPStorm integration (setting the parameter doesn't do anything).
  • fixed fallback DFS configuration for consistency's sake, settings should be loaded from project config anyway
  • removed redundant fallback storage path container parameter
  • fixed malformed ibexa.persistence.legacy.dsn parameter value

QA

All issues reported in IBX-3740 should be resolved by this. We need to focus on DFS configuration as it affects Flysystem v2 upgrade.

Scope: It affects both standalone installation and Platform.sh.

Checklist:

  • Provided PR description.
  • Tested the solution manually (local, not p.sh).
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review

@sonarcloud
Copy link

sonarcloud bot commented Dec 21, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alongosz alongosz marked this pull request as ready for review December 22, 2022 00:06
@alongosz alongosz requested a review from a team December 22, 2022 00:11
@micszo micszo self-assigned this Dec 22, 2022
Copy link
Contributor

@micszo micszo left a comment

Choose a reason for hiding this comment

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

A summary of my tests on Commerce 4.3:

  1. DFS/NFS worked for me on mysql and postgres with this fix. 👍🏻

  2. On Platform.sh with Elastic the elasticsearch_dsn parameter in my tests was still not set correctly with this fix.

I consulted with @vidarl and using his patch instead Elastic worked correctly on p.sh.

I think the three of us need to sync next week, either I am missing something or we are missing something here.

@adriendupuis
Copy link
Contributor

Hi,

While testing on Experience 4.4 and 4.5 (not 4.3), DFS_NFS_PATH environment variable (server side, not dotenv) isn't passed properly to DFSSiteAccessAwarePathPrefixer and is override by the default value. This PR merged to respectively 4.4 and main branches fixes the issue.

@konradoboza konradoboza changed the title IBX-3740: Prepended default Core settings [WiP] IBX-3740: Prepended default Core settings Mar 12, 2024
@konradoboza konradoboza changed the base branch from main to 4.5 March 12, 2024 13:50
Copy link

sonarcloud bot commented Mar 12, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@konradoboza konradoboza changed the title [WiP] IBX-3740: Prepended default Core settings IBX-3740: Prepended default Core settings Mar 22, 2024
@konradoboza
Copy link
Contributor

@micszo ibexa/recipes-dev#112 should be enough to fix the issue with elasticsearch not working on platform.sh. This PR was in turn only rebased against 4.5 and can be picked up for a retest.

Copy link
Contributor

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Tested with DFS/NFS. Setup via env vars works fine now.

QA Approved on Ibexa Commerce 4.5.7-dev.

@micszo micszo removed their assignment Mar 27, 2024
@alongosz alongosz merged commit 0334d52 into 4.5 Mar 27, 2024
22 checks passed
@alongosz alongosz deleted the ibx-3740-fix-dfs-config branch March 27, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working QA approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.