-
Notifications
You must be signed in to change notification settings - Fork 1
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: publish index claim #22
Conversation
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.
General LGTM. Let's get test and maybe a bit more commentary on logic.
pkg/service/contentclaims/service.go
Outdated
}, | ||
), | ||
assert.Location.Can(): server.Provide( | ||
assert.Location, | ||
func(cap ucan.Capability[assert.LocationCaveats], inv invocation.Invocation, ctx server.InvocationContext) (assert.Unit, receipt.Effects, error) { | ||
log.Errorf("TODO: implement me") | ||
return assert.Unit{}, nil, nil | ||
err := indexer.PublishClaim(context.TODO(), inv) |
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.
This would be for when we publish our own claims for our own service right?
Cause seperately we have the "CacheClaim" call just for the location commitments manged by others.
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.
sorry rewind looks like neither this nor the equals claim are implemented?
break | ||
} | ||
if ferr != nil { | ||
return fmt.Errorf("fetching blob index: %w", err) |
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.
it seems like if we had a successful fetch, even if another errored, then we can proceed, no?
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.
Oops! Good catch.
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.
No wait, a successul fetch will set ferr
to nil
, so we will proceed.
also you're failing tests at the moment. |
5c65928
to
e467ac1
Compare
remove a panic and get test passing in content claims service for now.
fa74036
to
6870663
Compare
use extracted go-capabilities library
use extracted go-metadata package
implement equals claim publishing
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
TODO: add tests