EXPERIMENTAL: Refactor

[rhysrevans3]

Description: @jonhealy1 I liked what you'd done with the sharing of code between elasticsearch and opensearch in core and wondered if it could be taken further.

I've moved the database specific code to the database logic (only for elasticsearch so far). This removes all database specific code from core.py. If something similar could be done for the other backend this could be move to stac-fastapi. I think other parts like base_database_logic, extensions, models, and utils could also be moved/shared.

Possible benefits:

• reduced code duplication
• better separation between API and database layers
• more cross pollination between backends

I wanted to check this was something positive before I went to far down the rabbit hole.

PR Checklist:

• [ ] Code is formatted and linted (run pre-commit run --all-files)
• [ ] Tests pass (run make test)
• [ ] Documentation has been updated to reflect changes, if applicable
• [ ] Changes are added to the changelog

[jonhealy1]

Hi Rhys. I think the idea is to move code out of database logic and into core. This is to make it as easy as possible for someone to implement their own custom backend / database solution. Core is more like database-core.

If you look at one of the original stac-fastapi backends, stac-fastapi-sqlaclchemy is the best example, you can see that there is a fair amount of code that is almost identical to what we use here in core. The idea is to take the common code out of these backends and put them in one place so everything is easier to maintain.

If we move code to database logic from core then at some point we don't need core anymore and then we have reversed everything :) The idea is that only elasticsearch-specific code would be in database_logic. I am sure there is a lot we could do to improve on this idea. Maybe better names too could help for the files.

[jonhealy1]

I was thinking a little bit about moving this to the core library - https://github.com/stac-utils/stac-fastapi-sqlalchemy - it used to be popular but now it is not even being maintained.

[rhysrevans3]

That's a very good point and I agree.

I don't think I put my idea across very well and it's probably better to separate into two points.

1. The main point, which is sound like you've already been thinking of doing, was to move /core to a core library. Is there a reason why this is stac-fastapi-sqlalchemy and not stac-fastapi
2. Database level shared code should be in base_database_logic.py (not database logic) rather than core.py. I think this give better the separation between API and database and makes it easier to override for a specific backend if needed. But this is probably more of a design pattern and just a personal preference.

Closing this pull request as I think we're on the same page with the important part 👍

[jonhealy1]

Moving shared code to base_database_logic may be a really good idea. I am not sure what you asking about stac-fastapi-sqlalchemy and stac-fastapi.

[jonhealy1]

Originally there was just the stac-fastapi repo. That repo had two backends - pgstac and sqlalchemy. Afterwards we created stac-fastapi-elasticsearch which borrowed from stac-fastapi but lived in a separate repo. Later the main developers moved the pgstac and sqlalchemy code out of the main stac-fastapi repo into separate repos. The thing with all of the backends is that they share a lot of the same logic. You may know all of this already.

[jonhealy1]

The logic that all of the backends share could maybe be moved back to the main stac-fastapi repo someday

[rhysrevans3]

Ah I didn't know the history but that explains the shared logic thank you.

I was thinking a little bit about moving this to the core library - https://github.com/stac-utils/stac-fastapi-sqlalchemy - it used to be popular but now it is not even being maintained.

I thought this might have meant that the shared code would live in stac-fastapi-sqlalchemy so wanted to just double check that it would be the main stac-fastapi repo that shared code would be moved too.