Increase core code?

[rhysrevans3]

This might be more of a discussion point that an issue. But it seems to me there is a lot of duplication between the different backends. Would it be possible to harmonise them. I know @jonhealy1 done something similar with the Elasticsearch and Opensearch parts of STAC FastAPI Elasticsearch Opensearch but I wondered if this was extendable to the other backends.

Possible areas of shared code:

• Base Database model. Having a common set of methods, with typed input and outputs, for the database would make code sharing of other areas easier.
• Switching from abstract methods to default methods for the BaseCoreClients. This would mean moving backend specific code to the database level which would give better separation of the API and Database layers. These methods could still be overridden in the backend if needed.
• Base app.py. Most of this code looks like it could be shared if extensions can be split into API and Database components.
• Authentication. Authentication should be independent of the chosen database.
• Utilities. Some utility methods are copied across multiple backends and I imagine there are others that could be shared.

Ideally the only parts exclusive to the backends should be the database, serialisation, settings, and the database components of extensions. I understand this level of code sharing may not be possible.

This would require substantial changes to the core and backend code bases so I wanted to see if others agreed or if the backends were too different.

Tagging some people I think might have opinions please share with others.

@vincentsarago @jonhealy1 @m-mohr

[vincentsarago]

@rhysrevans3 I'm not sure how it would look like to be honest.

maybe @bitner and @gadomski would be interested by this topic as well

[gadomski]

It's an interesting question. In general, the more we can abstract common behaviors to this repo, the better, so 👍🏼 to the core concept ... it's the implementation that gets sticky.

For background, the different backends used to live in this repo, which made it easier to abstract common patterns at the cost of higher maintenance burden. In the first half of 2023 we separated the backends into their own repos: #517.

I personally just accept that the core stac-fastapi will probably remain smaller w/ some duplicated behaviors in the backends because the domain entities, abstractions, etc are all a bit confused. But any efforts to clean things up are of course welcome!

[rhysrevans3]

Thanks @vincentsarago & @gadomski I thought there was probably a good reason for this. Just to give a quick example of how I imagined the code might look.

class AsyncCoreClient(LandingPageMixin):
    database: AsyncDatabaseClient = attr.ib()
    item_serializer: ItemSerializer = attr.ib()

    async def get_item(self, item_id: str, collection_id: str, **kwargs) -> stac_types.Item:
        item = await self.database.get_item(item_id=item_id, collection_id=collection_id)
        return self.item_serializer.db_to_stac(item)

@attr.s
class AsyncDatabaseClient(abc.ABC):
    
    @abc.abstractmethod
    async def get_item(self, item_id: str, collection_id: str) -> dict:
        ...

Search could be a bit more complex. @jonhealy1 has used an "append" style where each extension adds to the search query before it's run in the elasticsearch/opensearch shared code. But I not sure if this is compatible with the other backends?

I understand the relative gain might not be worth the amount of effort it would take to implement across the backends. And it might just shift the issue into the database. But I wanted to see the overall opinion was to it and I'm happy to create a pull request with a DatabaseClient and CoreClient if it would be useful?

[vincentsarago]

To be honest I'm not quite sure to see the difference between what we already have 😬

IMO there are already too many abstraction level in stac-fastapi (stac models, StacApi, Client) which makes customization of the application itself quite hard.

If I understand well what you are proposing we'll endup with another layer of abstraction

• Models
• Client
• Database
• StacApi

[rhysrevans3]

I agree it is similar to what what is already there.

I would say it was swap rather than addition of abstractions. My suggestion would remove client abstract base class and replace it with a single core client that is used by all databases (maybe this still amounts to an abstraction). And to add a database abstract base class that has a set of methods that the core client would use.

But I understand your point that this could make customisation even more difficult and if we're happy with the current architecture I'm happy to leave it as is.