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

feat(compiler): CRDT-maps implementation #1142

Merged
merged 91 commits into from
Jun 6, 2024
Merged

Conversation

DieMyst
Copy link
Member

@DieMyst DieMyst commented May 22, 2024

Description

Implementation of CRDT-maps in Aqua

Proposed Changes

https://www.notion.so/fluencenetwork/CRDT-Maps-in-Aqua-82d047246329446b8e1ca6f4db73228b?pm=c

Checklist

  • Corresponding issue has been created and linked in PR title.
  • Proposed changes are covered by tests.
  • Documentation has been updated to reflect the changes (if applicable).
  • I have self-reviewed my code and ensured its quality.
  • I have added/updated necessary comments to aid understanding.

Reviewer Checklist

  • Tests have been reviewed and are sufficient to validate the changes.
  • Documentation has been reviewed and is up to date.
  • Any questions or concerns have been addressed.

@DieMyst DieMyst added the e2e Run e2e workflow label May 22, 2024
streamMap <<- "key one", "new"
streamMap <<- "key two", ""
resSecond = streamMap.keysStream()
<- resEmpty, resFirst, resSecond
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not we join on result streams?


for r <- relays par:
on r:
join map.get("time")[relays.length - 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is not it join on an array that has no effect?

getKeys(mapName, mapType)
case (KeysStream, Nil) =>
getKeysStream(mapName, mapType)
case (n, args) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put this argument flattening inside getModel? It already flattens the id if it is a string literal. And contains uses getModel inside, so there will be no need to use it twice.

@@ -129,7 +129,7 @@ trait TypesAlgebra[S[_], Alg[_]] {

def typeToCollectible(token: Token[S], givenType: Type): OptionT[Alg, CollectibleType]

def typeToStream(token: Token[S], givenType: Type): OptionT[Alg, StreamType]
def typeToStream(token: Token[S], givenType: Type, isMap: Boolean): OptionT[Alg, MutableStreamType]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Personally, I don't like such bool flags that essentially separate two distinct methods inside.
Here it is just typeToStream(...): ...[..., StreamType] and typeToStreamMap(...): ...[..., StreamMapType].

@DieMyst DieMyst enabled auto-merge (squash) June 6, 2024 03:41
@DieMyst DieMyst merged commit 934c20c into main Jun 6, 2024
11 checks passed
@DieMyst DieMyst deleted the stream-maps-implementation branch June 6, 2024 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e Run e2e workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants