openedx / platform-roadmap

Tracking the maintenance, enhancement, and advancement of the Open edX project.
11 stars 0 forks source link

Deprecation and Removal of site_configuration #21

Closed e0d closed 6 months ago

e0d commented 2 years ago

Narrative

Removing site_configuration, and all references to it across our platform, will increase the speed with which developers can work and reduce cognitive overhead by not having to worry about catering to a little-known, little-used feature. The feature that SC was meant for - microsites/whitelabel - are a concept that edX.org no longer uses or supports.

Goal Remove all references to site_configuration throughout the edX codebase (including edx-platform and other IDAs) to increase developer productivity and decrease cognitive overhead and maintenance.
Reach (Customer) edX and Open edX developers will be impacted by the removal of our legacy site_configuration code, which is littered throughout various codebases. This will speed development and reduce cognitive overhead of writing code.
Impact + Measure (Outcome) Impact: Estimate that a block of code in a larger service/package like this requires ~5-6 weeks/year of maintenance costs.Faster development timelines and improved diagnosis of content issues will be possible by simplifying platform areas that are often poorly or not understood by internal teams.
T&L Gap Impact(FY21 Gap list) This work is not targeting a specific T&L gap, and is more focused on developer pain and development speed.

OmarIthawi commented 2 years ago

Thanks @e0d for sharing this.

Is there a way in which we can prevent or delay this deprecation? I'm thinking of few releases after Nutmeg until we can provide a better alternative for a large architectural change.

I understand that there's a cost of ownership and we should probably discuss the consequences of delaying such change.

A number of Open edX providers including Appsembler and eduNEXT uses this feature extensively. This also breaks Figures multi-site mode.

In the long term, we plan also to move away from the site_configuration, but as one can imaging it's much more involved to move out such change when its at the core of our systems.

Here are the alternative I'm thinking of:

Thanks @idegtiarov for scheduling this in the contributor meeting (https://github.com/openedx/community-wg/issues/55). I'm interested to hear from both of you and @felipemontoya about the deprecation of this feature and what are your plans.

sarina commented 2 years ago

It would be great if this could be filled out as a Public Engineering DEPR ticket, so that it follows the template and has as much info as possible. @e0d would your be willing to do that? https://github.com/openedx/public-engineering/issues/new/choose

This is per recent updates to OEP-21.

e0d commented 2 years ago

@sarina I am the creator of this only in the sense that I publicized someone else's proposal. This was originally pitched as BD-47. And, I don't see a ticket on the DEPR board for this, though that was request in the brief as a follow up action.

Arch-BOM was listed as the stakeholder for this proposal on the brief. @jmbowman may have some additional context.

I agree it should be represented on the public DEPR board, but don't think I'm the right person to create the issue.

@OmarIthawi I don't know if there are other stakeholders with reasons to remove this sooner than later, however, I'm glad we are having this discussion. From my point of view the timeline is flexible. Ideally we would have a DEPR ticket and this conversation could happen on it.

As a side note, are AppSembler interested in maintaining site_configuration?

sarina commented 2 years ago

Thanks @e0d - the clarification is appreciated.

I wrote the Blended brief, but I did abandon it well before we got to the part about talking to providers, as I was having trouble finding available tech leadership from the appropriate group. The first task of the pitched Blended project was to create the DEPR ticket itself, and begin gathering community feedback so we could understand how to chart the course of the project. I think beginning this discussion now is more than prudent, as we do have many steps to get through, and getting community input is high up on the list.

felipemontoya commented 2 years ago

I recently talked to @cmltaWt0 about this in the BTR channel. I have known about this since it was published, but we are actually not very heavy users of this feature. Our multi-tenant approach moved some years ago to a solution built in a custom plugin for this called eox-tenant.

Our solution came to be because we were tired of adding site_configuration support for features in the core one by one. Given that site_config has always been a second class citizen of the edx-platform repo I be happy to help us move into a direction that lets plugins support what site_configuration did and even take it further without requiring the core platform to be mindful of this all the time, specially for developers who don't use the feature. By further I mean things that have never been well covered by site_config such as waffle flags or configuration models.

I'm happy to continue this conversation on the depr ticket .

feanil commented 2 years ago

Looks like this was not added to the DEPR board, I've marked this as proposed so that we can start putting it through the DEPR process. @e0d is there someone already driving this work? That the DEPR group should work with?

dianakhuang commented 1 year ago

Discussing in the DEPR working group, this functionality is de-facto deprecated that in new systems/MFE are not supporting site-awareness. This is something we will want to be thoughtful about as we get alternatives in place.

e0d commented 1 year ago

@OmarIthawi What's your current interest in this feature?

OmarIthawi commented 1 year ago

Thanks for checking @e0d. We're in the process of moving away from the SiteConfiguraiton models. We're not quiet there yet.

For example, we still depend on the openedx.core.djangoapps.site_configuration.helpers.get_value helper which helps us to override a lot of core configuration values without patching the platform itself.

If it's possible, I suggest the we remove the SiteConfiguration in multiple iterations:

Release 1 -- first batch of breaking changes:

Release 2 -- second batch of breaking changes:

I think this strikes a good balance in removing a rarely used functionality (although we do use it), while also avoid the need for a sudden major refactor.

The main driver for such iterative deprecation is the enormous references for helpers.get_value():

openedx/edx-platform (@ master) $ git grep get_value -- '*.py' | wc -l
447

Please let me know what do you think؟

robrap commented 1 year ago

Adding a link to some docs for reference when and if this moves forward: https://edx.readthedocs.io/projects/edx-installing-configuring-and-running/en/latest/search.html?q=site+configuration&check_keywords=yes&area=default

robrap commented 1 year ago

Was DEPR ever announced? Also, it is missing a lot of the metadata we collect for actual DEPR tickets. I'm asking because https://github.com/openedx/edx-platform/pull/32656 just came up, and it would be useful to know how to proceed. FYI: @e0d @dianakhuang

feanil commented 1 year ago

I think the current state is that it's desired but no one can drive this forward. In this case I think it makes sense to allow for improvements since we don't have alternatives that we can provide to the community.