Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break circular dependencies #4783

Closed
wants to merge 22 commits into from

Conversation

olivergrabinski
Copy link
Contributor

No description provided.


trait FetchResource {

def latest(ref: ResourceRef, project: ProjectRef): FetchF[Resource]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is not only the latest but possibly a revision or a tag too


def apply(xas: Transactors): FetchResource = (ref: ResourceRef, project: ProjectRef) =>
ScopedStateGet
.latest(Resources.entityType, project, ref.iri)
Copy link
Contributor

@imsdu imsdu Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on the case you may want to resolve any type of resource or a specific one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this imply we should have a function in ScopedStateGet that allows fetching the state without specifying the entity type?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, we have to focus on the one related to resources/schemas, the general one is only used for resolver endpoints afaik so we should just leave it as it is for now as we don't need it for the batch

import doobie.implicits._
import doobie.{Get, Put}

trait FetchResource {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could declare it in the distage module and use directly where it is used instead of passing along the transactors which is a bit low level ?
Any blocker to that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should be ok yes

@@ -62,7 +62,7 @@ class ResolverContextResolutionSuite extends NexusSuite {
)
)

def fetchResource: (ResourceRef, ProjectRef) => FetchResource[Resource] = { (r: ResourceRef, p: ProjectRef) =>
def fetchResource: (ResourceRef, ProjectRef) => FetchF[Resource] = { (r: ResourceRef, p: ProjectRef) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the tests, you could have the FetchResource type too

.query[S]
.option

def apply[Id: Put, S: Get](tpe: EntityType, project: ProjectRef, id: Id, rev: Int): ConnectionIO[Option[S]] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't store the state for every revision, we have to query for events and replay them in a Stream (the code is already in ScopedStateStore, it could be moved here)

@olivergrabinski olivergrabinski marked this pull request as ready for review March 8, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants