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

NewContext should use tiledb_ctx_alloc_with_error #223

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Shelnutt2
Copy link
Member

This allows capturing any errors on alloc of the context.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #18266: Use tiledb_ctx_alloc_with_error to capture errors.

@Shelnutt2 Shelnutt2 force-pushed the sethshelnutt/sc-18266/use-tiledb-ctx-alloc-with-error-to-capture branch from 8657c26 to 5eee8cd Compare June 1, 2022 18:14
context.go Outdated Show resolved Hide resolved
This allows capturing any errors on alloc of the context.
@Shelnutt2 Shelnutt2 force-pushed the sethshelnutt/sc-18266/use-tiledb-ctx-alloc-with-error-to-capture branch from 5eee8cd to 8c27f0b Compare June 1, 2022 19:01
@@ -4,6 +4,7 @@ package tiledb
#cgo LDFLAGS: -ltiledb
#cgo linux LDFLAGS: -ldl
#include <tiledb/tiledb.h>
#include <tiledb/tiledb_experimental.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is experimental, do we need to have a separate "experimental" vs. "non-experimental" file for this, with a flag like this:

//go:build experimental

(and a corresponding "not experimental" file)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well this API is experimental mostly because we don't know why allocs fail. Once we collect some of the errors that will inform us about the reasons and lead to possible API changes in which we might want to remove or change this with_error API.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is about separating things which use "experimental" TileDB APIs versus the stable ones, and the assumption that I assume we make that only stable APIs should be included when building without -tags=experimental. I have an example of what I mean by this here: 53e5e08

@Shelnutt2
Copy link
Member Author

@snagles we should revist this to start collecting the errors.

@teo-tsirpanis teo-tsirpanis requested a review from a team as a code owner November 25, 2024 17:10
@teo-tsirpanis teo-tsirpanis removed the request for review from anastasop November 25, 2024 17:11
@teo-tsirpanis
Copy link
Member

CI fails because tiledb_ctx_alloc_with_error is not exported in the binaries. This is expected to be solved once we move to 2.27.

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.

3 participants