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

store and write proxy config to wp-config.php #171

Merged
merged 16 commits into from
Nov 30, 2023

Conversation

Thanhphan1147
Copy link
Contributor

@Thanhphan1147 Thanhphan1147 commented Nov 23, 2023

During _reconciliation the charm checks if the themes and plugins currently installed matches the expected themes and plugin. If there are some missing the charm will then download them from https://wordpress.org. Behind the scene it runs wp theme|plugin install and If the charm is deployed behind a proxy and the proxy information is not present in wp-config.php this command will fail, leaving the charm in a blocked state.

This PR addresses that issue by fetching the proxy information from juju model-config and populate the proxy-config attribute of the charm state so that the values can be added to wp-config.php during reconciliation.

Checklist

@Thanhphan1147 Thanhphan1147 marked this pull request as ready for review November 27, 2023 18:02
@Thanhphan1147 Thanhphan1147 requested a review from a team as a code owner November 27, 2023 18:02
@amandahla
Copy link
Contributor

Could you fill the PR description please? Out of curiosity: whats the impact if the proxy is not reachable? Is Wordpress still up?

@Thanhphan1147 Thanhphan1147 added the bug Something isn't working label Nov 27, 2023
@Thanhphan1147
Copy link
Contributor Author

Hi, Thanks for the notice, I have updated the PR description with relevant information. Basically wordpress will be in a blocked state if the plugins|themes cannot be installed

@jdkandersson
Copy link
Contributor

Technically the checkbox for " The PR is tagged with appropriate label (urgent, trivial, complex)" is checked though the PR is not labelled with any of those tags, could that please be resolved?

src/state.py Outdated Show resolved Hide resolved
tests/unit/conftest.py Outdated Show resolved Hide resolved
Copy link
Contributor

Test coverage for 7c08317

Name                Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------
src/charm.py          546     41    180     32    89%   191-194, 358-359, 418->422, 571, 602, 608, 653, 688-689, 740-747, 752, 854->859, 858, 860, 865-866, 926, 944, 951, 1041, 1050, 1062, 1083, 1092, 1111, 1115, 1144, 1197, 1329, 1351, 1358->1360, 1403->exit, 1415, 1431, 1468, 1477-1478
src/cos.py             15      0      0      0   100%
src/exceptions.py      17      1      2      1    89%   41
src/state.py           34      0      4      0   100%
src/types_.py          16      0      0      0   100%
---------------------------------------------------------------
TOTAL                 628     42    186     33    90%

Static code analysis report

Run started:2023-11-28 16:52:15.068250

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 4428
  Total lines skipped (#nosec): 1
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

Copy link
Contributor

@amandahla amandahla left a comment

Choose a reason for hiding this comment

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

LGTM

@Thanhphan1147 Thanhphan1147 added the complex This is a complex PR that requires more senior attention label Nov 29, 2023
@Thanhphan1147
Copy link
Contributor Author

Thanhphan1147 commented Nov 29, 2023

Technically the checkbox for " The PR is tagged with appropriate label (urgent, trivial, complex)" is checked though the PR is not labelled with any of those tags, could that please be resolved?

I labelled the PR appropriately, I also removed the bug label put by mistake

@Thanhphan1147 Thanhphan1147 removed the bug Something isn't working label Nov 29, 2023
@Thanhphan1147 Thanhphan1147 merged commit 16b8c7d into main Nov 30, 2023
35 checks passed
@Thanhphan1147 Thanhphan1147 deleted the store_and_write_proxy_config branch November 30, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complex This is a complex PR that requires more senior attention Libraries: Out of sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants