edx-platform icon indicating copy to clipboard operation
edx-platform copied to clipboard

Published 20 hours ago • verified publisher icon openedx

[DEPR]: Neo4J Support

Open dianakhuang opened this issue 11 months ago • 10 comments

Proposal Date

2024-03-07

Target Ticket Acceptance Date

2024-03-21

Earliest Open edX Named Release Without This Functionality

Sumac 2024-10

Rationale

We would like to take out neo4j support from edx-platform and remove it as a dependency from edx-platform. We believe this library and its usage is underused and not particularly valuable as many operators have switched to searching course data in other types of data stores.

Removal

Removal of https://github.com/openedx/edx-platform/tree/master/cms/djangoapps/coursegraph and the neo4j library from edx-platform. If anyone wants to keep this functionality, a separate plugin can be created.

Replacement

We do not have a replacement.

Discuss thread: https://discuss.openedx.org/t/deprecation-removal-coursegraph-neo4j-support/12480

dianakhuang avatar Mar 07 '24 18:03 dianakhuang

For folks looking for a replacement, Aspects we've emulated much of what Coursegraph does in ClickHouse via openedx-event-sink-clickhouse (which currently has a DEPR, but just to consolidate the code with other Aspects plugins in platform-plugin-aspects). It's not 1:1, but can help answer some of the same questions.

bmtcril avatar Mar 07 '24 21:03 bmtcril

Assign it to me

qasimgulzar avatar Mar 25 '24 04:03 qasimgulzar

Here I have created a PR Please review

qasimgulzar avatar Mar 25 '24 05:03 qasimgulzar

@bmtcril could you please review my PR?

qasimgulzar avatar Mar 25 '24 17:03 qasimgulzar

According to @dianakhuang 's DEPR this is supposed to remain through Sumac. I assume to let the community have one release of warning before the removal. @qasimgulzar are you proposing to merge this sooner?

bmtcril avatar Mar 25 '24 17:03 bmtcril

As of it doesn't seems like openedx itself consuming neo4j. I believe it is safe to merge.

qasimgulzar avatar Mar 29 '24 10:03 qasimgulzar

Hi @qasimgulzar I'm saying that merging the PR now goes against the DEPR announcement and the PR description since it which we can't allow since "Earliest Open edX Named Release Without This Functionality" would actually be Redwood. I'm not against removing it before Redwood, but we should follow the process and update the DEPR and re-announce the new dates to make sure that anyone using it will know that they need to make adjustments sooner.

bmtcril avatar Mar 29 '24 12:03 bmtcril

I don't personally think this would be prudent given the small amount of time before Redwood cut, but that is just my opinion.

jristau1984 avatar Mar 29 '24 14:03 jristau1984

I think it make sense to give it more time, I can make another PR to add deprecation warning for this app before removing.

qasimgulzar avatar Mar 29 '24 14:03 qasimgulzar

Here I have created a new PR to add warning message.

qasimgulzar avatar Apr 01 '24 09:04 qasimgulzar

The removal has landed on master of edx-platform via https://github.com/openedx/edx-platform/pull/34417

feanil avatar May 28 '24 14:05 feanil