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

Maintenance Container for asynchronous tasks #1

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

PiyushRaj927
Copy link

@PiyushRaj927 PiyushRaj927 commented Mar 27, 2023

Related issue Canasta#33
Related PRs Canasta#266, Canasta-DockerCompose#42
This PR adds a separate container to run maintenance script for MediaWiki installation

changes

The changes have been made using the base code from canasta and include the following modifications:

  • Removal of Apache from Dockerfile and run-apache.sh along with related configuration files
  • Updating the startup script to run the maintenance scripts as child processes
  • Updating of the GitHub workflow (not tested)

testing

  • Below is the process structure for the maintainance container
# pstree
docker-init---run-apache.sh-+-run-apache.sh---runuser---mwjobrunner.sh---sleep
                            |-run-apache.sh---runuser---mwtranscoder.sh---sleep
                            `-run-apache.sh---runuser---mwsitemapgen.sh---sleep

The sitemap script is disabled by default; It was enabled for testing purposes.

  • The following parameters are modified in docker-image.yml from the Canasta repository to make it work for this repository
    • concurrency
    • on.push.branches
    • env.IMAGE_NAME
    • Image_ID
      I'm unfamiliar with GitHub workflow, so I cannot be sure if I have covered everything. Please let me know if I have missed something

I have tested the changes made by importing an existing wiki as well as creating a new one

brave_03-28-23(18-53-333)
brave_03-28-23(18-53-062)

PS- to locally test please edit the docker-compose.override.yml to point to the local images as there is no image in registry for this container

/**
* Not exactly a utility function, but - show a warning to users if $wgSMTP is not set.
*/
$wgHooks['SiteNoticeAfter'][] = function ( &$siteNotice, Skin $skin ) {
Copy link
Member

Choose a reason for hiding this comment

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

Not needed in maintenance container

@@ -0,0 +1,45 @@
RewriteEngine On
Copy link
Member

Choose a reason for hiding this comment

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

The entire file is not necessary in this container and should be removed

@@ -0,0 +1,149 @@
vcl 4.0;
Copy link
Member

Choose a reason for hiding this comment

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

This entire file is unnecessary in this container

@@ -0,0 +1,10 @@
User-agent: *
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary file

@PiyushRaj927
Copy link
Author

I have deleted the files as per the request.
Please review them.

@PiyushRaj927 PiyushRaj927 requested a review from jeffw16 March 29, 2023 16:17
@PiyushRaj927
Copy link
Author

PiyushRaj927 commented Mar 30, 2023

Hi @jeffw16, I was wondering if you had a chance to review my edits, and if there's anything I can do to help move the task forward. Please let me know if any further changes are required. Thanks

@jeffw16
Copy link
Member

jeffw16 commented Apr 2, 2023

No, I have not had a chance to review your edits again. Sorry for taking so long; your patience is appreciated.

In case you are wondering if it looks bad to have a PR not merged in for being selected as a GSoC contributor: no, it's not. We're not counting it against any candidate who has a reasonable first draft of their code submitted in a PR. :)

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.

2 participants