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: publish index claim #22

Merged
merged 9 commits into from
Oct 22, 2024
Merged

feat: publish index claim #22

merged 9 commits into from
Oct 22, 2024

Conversation

alanshaw
Copy link
Member

  • Wires up the Ucanto endpoints to publish claims when invoked
  • Adds implementation for publishing index claims

TODO: add tests

@alanshaw alanshaw requested a review from hannahhoward October 14, 2024 15:57
Copy link
Member

@hannahhoward hannahhoward left a 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.

},
),
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)
Copy link
Member

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.

Copy link
Member

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)
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops! Good catch.

Copy link
Member Author

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.

@hannahhoward
Copy link
Member

also you're failing tests at the moment.

@hannahhoward hannahhoward changed the base branch from refactor/remove-repeated-service-interface to main October 14, 2024 20:54
@hannahhoward hannahhoward force-pushed the feat/publish-index-claim branch from 5c65928 to e467ac1 Compare October 14, 2024 20:57
@hannahhoward hannahhoward force-pushed the feat/publish-index-claim branch from fa74036 to 6870663 Compare October 22, 2024 02:49
Copy link
Member

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

LGTM

@hannahhoward hannahhoward merged commit 12c4721 into main Oct 22, 2024
6 checks passed
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