-
Notifications
You must be signed in to change notification settings - Fork 2
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
build(deps): upgrade EDC to 0.1.2 #35
Conversation
9798443
to
151409f
Compare
I'm working to switch all the calls to the new Json-LD endpoints, unfortunately seems that the will see if there are workaround to this |
bf7cdbb
to
0718110
Compare
* chore: update old asset entities specs & add new context * chore: use new connector entities in management controllers and utils * test: adjust asset tests * chore: update entities * fix: fix contract, policy and asset failing tests * chore: update addresses props in connector config files * refactor: Standardize request body objects and update entities * fix: update catalog and contract negotiations tests * Fix CI --------- Co-authored-by: ndr_brt <[email protected]>
@ndr-brt I took the liberty of removing the ternary operator in my latest commit because it's not really doing anything there. The initial value assigned to the query is |
seems like negotiations got TERMINATED instead of FINALIZED, now I improved the |
4ef0d6f
to
3d9cb72
Compare
I think it has something to do with the json-ld expansion, because this offer: {
"@id": "ZGVmaW5pdGlvbi0zZjgzNTFmMS0zOWQ0LTQ1ODMtYmI2YS0yOWZjZDg0NGJhOTg=:NDQ0MTlmMzEtNTFkMi00MDE4LWJiNDYtYWFjMjczMmM5MWFl:M2E5MDExNTQtM2U0Yy00N2FhLWFhZjYtNDQxMmMwOTY0MDQ0",
"@type": "odrl:Set",
"odrl:permission": [],
"odrl:prohibition": [],
"odrl:obligation": [],
"odrl:target": "44419f31-51d2-4018-bb46-aac2732c91ae"
}
gets expanded as: {
"id": "NDQ0MTlmMzEtNTFkMi00MDE4LWJiNDYtYWFjMjczMmM5MWFl:M2E5MDExNTQtM2U0Yy00N2FhLWFhZjYtNDQxMmMwOTY0MDQ0",
"type": [
"http://www.w3.org/ns/odrl/2/Set"
],
"http://www.w3.org/ns/odrl/2/obligation": [],
"http://www.w3.org/ns/odrl/2/permission": [],
"http://www.w3.org/ns/odrl/2/prohibition": [],
"http://www.w3.org/ns/odrl/2/target": [
{
"@value": "44419f31-51d2-4018-bb46-aac2732c91ae"
}
]
} so it loses the first part of the ID, could be related to an issue that I already opened on: No clue why this is not happening locally... |
f41236c
to
29269e1
Compare
@OlfaBensoussia the ternary operator is necessary, because an empty json-ld cannot be expanded and it returns error, so the body should be either null or a valid json-ld (if you remove that now the related test will fail). Now we're green with 0.1.2, for 0.1.3 we'd need to discover why the expansion of the ready for merge and release? @fdionisi |
29269e1
to
4e6da0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stunning job, @OlfaBensoussia and @ndr-brt. I have just a few nits, but most of them could be handled as follow-ups. Let me know your thoughts.
20220b6
to
57c83f7
Compare
57c83f7
to
a684f77
Compare
src/entities/contract-agreement.ts
Outdated
providerAgentId?: string; | ||
|
||
get assetId(): string { | ||
return this['https://w3id.org/edc/v0.0.1/ns/assetId'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndr-brt do you think it will make sense if we tried to use ${EDC_CONTEXT}assetId
instead of always hardcoding the value? it seems like there is a pattern in getting those values in general, it's usually 'https://w3id.org/edc/v0.0.1/ns/entityName'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey, I'm working at this, let's check out the latest commit 🌋
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to keep the objects not typed and defining properties through get
methods that take info out of the json-ld properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmmh... despite this is a really clean approach, typescript lacks this feature: microsoft/TypeScript#14417
and it's pretty fundamental for what I was doing here... maybe something that can be used in the meantime for "output" objects, instead for creating "input" objects better go with the current approach.
The intersection between the two is the Policy
class, that's also a crucial one.
adc823d
to
1ddbe01
Compare
6ab7e6d
to
71925b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
what
Upgrade EDC to
0.1.2
why
keep dependencies updated
notes
updated gradle at 7.6.1 as well