Remove unused components #302

mishaschwartz · 2023-03-22T15:13:03Z

Overview

The default stack contained components that were unmaintained and no longer used. This removes the following components from the stack:

frontend
project-api
catalog
solr
ncwms2
ncops

Changes

Non-breaking changes

Breaking changes

Removes the components listed above from the stack. Technically this change may not necessarily break any existing deployments since (I hope) none of the components listed above are being actively used anywhere.

Related Issue / Discussion

Resolves Migrate existing pieces to new pluggable component architecture, part 1 clean up the unused pieces #214
Closes Remove pavics-datacatalog from stack #287 (replaces it)
Resolves 💡 [Feature] Remove PAVICS-Datacatalog #206
Closes Remove ncwms #291 (replaces it)
Resolves Replace ncWMS2 by THREDDS WMS service #9
Closes Remove malleefowl server from stack #292 (replaces it)
Resolves 💡 [Feature] Remove malleefowl #290

Additional Information

This reverts commit 5b33aa5.

tlvu

Minor changes, the rest LGTM. Wait for my manual testing with Autodeploy before merging this.

tlvu · 2023-03-22T15:52:42Z

birdhouse/components/cowbird/default.env

+# (include this by default to support backwards compatibility for now)
+COMPONENT_DEPENDENCIES="
+  $COMPONENT_DEPENDENCIES
+  ./config/mongodb


Missed dependency from previous PR? :D

I also though Cowbird use its own MongoDB, did not realized it piggy-back on the shared MongoDB.

The problem is that it can use its own mongodb instance but by default it uses the one in config/mongodb.

I didn' t include it in the previous PR because it's not technically a dependency and it would break backwards compatibility because it would require the user to update their env.local files to include config/mongodb explicitly.

This PR is not backwards compatible so I put it in here but honestly I don't love this workaround. I think I'm going to change this for this PR too. Let me think about how to do this better and I'll update it shortly.

it would require the user to update their env.local files to include config/mongodb explicitly

But this change fixed that and ensure no manual edit of env.local is required, which make this change back-compat. The previous PR is actually not back-compat if Cowbird was activated alone.

During testing, we did not find out because probably another component pulled ./config/mongodb for Cowbird to use.

I would keep this as-is for now, unless you change the way Cowbird "ask for mongodb". This is more backward compat than the previous PR :)

Right.... though the previous PR was backwards compatible because the assumption was that every deployment was using all components in the original docker-compose.yml file (ie. the "default components") which included mongodb.

So yes, it would break if cowbird was activated alone but there would be no existing deployment that would ever do that.

tlvu · 2023-03-22T15:57:10Z

birdhouse/components/weaver/config/magpie/config.yml.template

@@ -158,12 +152,6 @@ permissions:
    permission: read
    group: anonymous
    action: create
-  # malleefowl


Oh maleefowl removed as well? Not listed in your Change.md.

tlvu · 2023-03-22T15:57:52Z

birdhouse/config/.gitignore

+frontend/
+malleefowl/
+ncwms2/
+phoenix/


phoenix removed too? I thought we would keep it.

We were going back and forth about keeping it. I guess we'll keep it for now though

tlvu · 2023-03-22T16:01:04Z

birdhouse/default.env

-  ./config/project-api
-  ./config/catalog
-  ./config/malleefowl
-  ./config/solr


Actually I don't know where this went...

This is really odd then, because ncwms2 was there after Autodeploy test in the previous PR ! Something else must have pulled it back in.

Frontend was pulling ncwms2 back in

birdhouse-deploy/birdhouse/config/frontend/default.env

Line 5 in 37fe32e

./config/ncwms2

Ah... ok then. Well frontend is gone in this PR too 🤷‍♂️

tlvu · 2023-03-22T16:03:51Z

birdhouse/scripts/bootstrap-instance-for-testsuite

-# Need to open temporary Thredds "testdata/secure/" on local PAVICS host to anonymous group.
-# Only crawl the subset enough to pass canarie-api monitoring
-# see config/canarie-api/docker_configuration.py.template
-$THIS_DIR/trigger-pavicscrawler target_files=birdhouse/testdata/flyingpigeon/cmip5


Oh no, this file should not be removed ! Only this pavicscrawler line and the sleep below need to be removed.

This script does a lot more than just populating the Solr DB.

tlvu · 2023-03-22T16:04:56Z

docs/source/data_catalog.rst

-
-
-
+Pavics offers intake `catalogs <pavics.ouranos.ca/catalog>`_ for different collections hosted on the THREDDS server. Loading those catalog requires the `Intake-ESM <https://intake-esm.readthedocs.io/en/stable/>`_ extension installed, together with Intake.


https:// in front of URL?

I think I copied this from another PR. I'll remove it if you want (but that is a valid URL)

Keep the text. I was simply referring to the URL. It looks wierd <pavics.ouranos.ca/catalog> without https:// in front.

The one below <https://intake-esm.readthedocs.io/en/stable/> has https in front. It just looks better to me.

crim-jenkins-bot · 2023-03-22T16:25:42Z

E2E Test Results

DACCS-iac Pipeline Results

Build URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1369/
Result : failure

BIRDHOUSE_DEPLOY_BRANCH : pluggable-components-part-1
DACCS_CONFIGS_BRANCH : master
PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master
PAVICS_SDI_BRANCH : master

DESTROY_INFRA_ON_EXIT : true
PAVICS_HOST : https://host-140-118.rdext.crim.ca

Infrastructure deployment failed. Instance has not been destroyed. @matprov

crim-jenkins-bot · 2023-03-22T19:48:22Z

E2E Test Results

DACCS-iac Pipeline Results

Build URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1371/
Result : failure

BIRDHOUSE_DEPLOY_BRANCH : pluggable-components-part-1
DACCS_CONFIGS_BRANCH : master
PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master
PAVICS_SDI_BRANCH : master

DESTROY_INFRA_ON_EXIT : true
PAVICS_HOST : https://host-140-216.rdext.crim.ca

Infrastructure deployment failed. Instance has not been destroyed. @matprov

mishaschwartz · 2023-03-23T15:31:21Z

@matprov Do you have any insight as to why the deployment is failing? I'm trying to debug with our local test server and everything looks good over here. Is there a jenkins config that I have to change to accommodate removing services?

fmigneault · 2023-03-23T16:33:15Z

@mishaschwartz
Please add me and @matprov as reviewers in for future PRs, just to make sure we are notified, as changes like this can affect other deployments than Ouranos. I only saw it because it was referenced explicitly in CanarieAPI's PR.

Personally, I would prefer that the following services/components are only disabled from the default set, but not entirely wiped-out from the repository:

catalog
malleefowl
ncwms2

The configurations combining Magpie/Cowbird/CanarieAPI and those services were very intricate to design. I would like to keep them around. There are also some references left around in tests/docs, so it is better to leave them visible for anyone digging for them.

Maybe add some badge as follows in a README under the config for those components:

Other components are safe to remove, as their projects are themselves archived or already replaced by something else.

mishaschwartz · 2023-03-23T17:50:34Z

@fmigneault

Thank you for letting me know. I'm basing my decisions on what to remove based on the many PRs and Issues that discuss these changes (some of which discuss in detail removing the components that you are proposing we keep):

If there is some further discussion that needs to happen regarding which components are still in use, let's have that discussion before this PR is merged.

I'll hold off on any more changes to this PR for now until we come to a final decision.

fmigneault · 2023-03-24T23:32:33Z

@mishaschwartz
I agree 100% with those issues regarding removing these components from the default set.
I simply want to "disable/deprecate" them, instead of wiping them out of existence.
They provide many useful configuration examples that can provide guidance or inspiration when adding future components.

tlvu · 2023-03-25T17:50:25Z

@mishaschwartz I think disabling them from the default enable list still achieve our goals of "removing" them from the stack. We do not have to physically delete their configs. Especially now that you have already spent the effort to split them out into proper components.

Some of these are fully functional with tests passing on Jenkins everyday (ex: old catalog and Solr). They are simply being replaced by the newer catalog.

I already hint that maybe we do not need to physically remove these deprecated components in this comment #296 (comment). But seeing you still want to delete what you just worked on, I said nothing.

In part 1 of this "make the stack fully modular" effort, I was going to fully delete these components because I did not want to spend time "componentize" deprecated items. Now you already "componentize" them, I think we could try to keep them unless they cause maintenance effort.

We can make it super clear these are deprecated items by moving them into legacy-components/ folder with a README stating clearly these are deprecated/replaced by newer components and are not maintained anymore or they could even be not functional. They are only being kept for archiving purposes.

In the case of ncops, it can fully be deleted since it was never been previously deployed.

mishaschwartz · 2023-03-27T14:05:10Z

Ok if we're moving, not removing then I'll handle this in an entirely different PR that will also include moving the other components around (out of the current components, config, and optional-components folders).

In that case I vote to close this PR. Feel free to reopen if you disagree and we can continue the discussion.

tlvu · 2023-03-27T14:59:45Z

I have to add that thanks to your hard work to componentize everything, we now have the option to keep the legacy components. Had you've followed the original plan, this would not have been possible. Thanks for coming up with alternative like this. It's great to have new ideas.

mishaschwartz added 12 commits March 22, 2023 10:51

remove catalog (and solr)

6a45923

remove ncwms2

94a8c8d

remove malleefowl

03e73ae

remove frontend

cd0aaeb

remove project-api

1562246

remove ncops

7c2befa

remove phoenix

5b33aa5

keep old gitignore for backwards compatibility

37ab063

remove solr from default conf dirs

4ebabaf

add mongodb as cowbird dependency for backwards comptibility

2a032dd

Revert "remove phoenix"

396e49a

This reverts commit 5b33aa5.

update CHANGES.md file

581cc2e

github-actions bot added the documentation Improvements or additions to documentation label Mar 22, 2023

tlvu requested changes Mar 22, 2023

View reviewed changes

minor fixes and restore bootstrap script

47409f4

mishaschwartz mentioned this pull request Mar 23, 2023

fix CanarieAPI config to support Python 3.x #284

Merged

fmigneault requested review from fmigneault and matprov March 23, 2023 16:33

mishaschwartz closed this Mar 27, 2023

mishaschwartz deleted the pluggable-components-part-1 branch April 12, 2023 22:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unused components #302

Remove unused components #302

mishaschwartz commented Mar 22, 2023 •

edited

Loading

tlvu left a comment

tlvu Mar 22, 2023

mishaschwartz Mar 22, 2023

tlvu Mar 22, 2023

mishaschwartz Mar 22, 2023

tlvu Mar 22, 2023

tlvu Mar 22, 2023

mishaschwartz Mar 22, 2023

tlvu Mar 22, 2023

mishaschwartz Mar 22, 2023

tlvu Mar 22, 2023

tlvu Mar 22, 2023

mishaschwartz Mar 22, 2023

tlvu Mar 22, 2023

tlvu Mar 22, 2023

mishaschwartz Mar 22, 2023

tlvu Mar 22, 2023

crim-jenkins-bot commented Mar 22, 2023

crim-jenkins-bot commented Mar 22, 2023

mishaschwartz commented Mar 23, 2023

fmigneault commented Mar 23, 2023

mishaschwartz commented Mar 23, 2023

fmigneault commented Mar 24, 2023

tlvu commented Mar 25, 2023

mishaschwartz commented Mar 27, 2023

tlvu commented Mar 27, 2023




		Pavics offers intake `catalogs <pavics.ouranos.ca/catalog>`_ for different collections hosted on the THREDDS server. Loading those catalog requires the `Intake-ESM <https://intake-esm.readthedocs.io/en/stable/>`_ extension installed, together with Intake.

Remove unused components #302

Remove unused components #302

Conversation

mishaschwartz commented Mar 22, 2023 • edited Loading

Overview

Changes

Related Issue / Discussion

Additional Information

tlvu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crim-jenkins-bot commented Mar 22, 2023

E2E Test Results

DACCS-iac Pipeline Results

crim-jenkins-bot commented Mar 22, 2023

E2E Test Results

DACCS-iac Pipeline Results

mishaschwartz commented Mar 23, 2023

fmigneault commented Mar 23, 2023

mishaschwartz commented Mar 23, 2023

fmigneault commented Mar 24, 2023

tlvu commented Mar 25, 2023

mishaschwartz commented Mar 27, 2023

tlvu commented Mar 27, 2023

mishaschwartz commented Mar 22, 2023 •

edited

Loading