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

UX improvements: FFI logging, error message fix, Rust stream borrow checks #515

Merged
merged 15 commits into from
Nov 3, 2023
1 change: 1 addition & 0 deletions c/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ rust_static_library(

"@crates//:chrono",
"@crates//:itertools",
"@crates//:env_logger",
"@crates//:log",
],
)
Expand Down
11 changes: 10 additions & 1 deletion c/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

use std::{cell::RefCell, ffi::c_char, ptr::null_mut};

use log::trace;
use env_logger::Env;
use log::{trace, warn};
use typedb_driver::{Error, Result};

use super::memory::{free, release_optional, release_string};
Expand All @@ -30,6 +31,14 @@ thread_local! {
static LAST_ERROR: RefCell<Option<Error>> = RefCell::new(None);
}

#[no_mangle]
pub extern "C" fn init_logging() {
const ENV_VAR: &str = "TYPEDB_DRIVER_LOG_LEVEL";
if let Err(err) = env_logger::try_init_from_env(Env::new().filter(ENV_VAR)) {
warn!("{err}");
}
}
Comment on lines +34 to +40
Copy link
Member Author

Choose a reason for hiding this comment

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

This function allows the FFI drivers to initialize the Rust logger. (Rust driver users can do this directly)


fn ok_record<T>(result: Result<T>) -> Option<T> {
match result {
Ok(value) => Some(value),
Expand Down
4 changes: 3 additions & 1 deletion c/swig/typedb_driver_java.swg
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

public static class Unchecked extends RuntimeException {
Unchecked(Error e) {
super(e);
super(e.getMessage());
}
}
%}
Expand All @@ -83,6 +83,8 @@
}
}

%nojavaexception init_logging;

/* simple getters do not throw */
%nojavaexception options_new;
%nojavaexception options_get_infer;
Expand Down
2 changes: 2 additions & 0 deletions c/swig/typedb_driver_python.swg
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ static PyObject* PyExc_TypeDBDriverError;

%pythoncode %{
TypeDBDriverExceptionNative = native_driver_python.TypeDBDriverExceptionNative

native_driver_python.init_logging()
%}

/* ValueType* needs special handling */
Expand Down
2 changes: 1 addition & 1 deletion dependencies/vaticle/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def vaticle_dependencies():
git_repository(
name = "vaticle_dependencies",
remote = "https://github.com/vaticle/dependencies",
commit = "67a0602be830227247d31bb881029d752f05f9bb", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_dependencies
commit = "d75f30de7902892b1d45015aefd20f06048b0458", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_dependencies
)

def vaticle_typedb_common():
Expand Down
3 changes: 3 additions & 0 deletions java/common/NativeObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
import com.vaticle.typedb.driver.common.exception.ErrorMessage;
import com.vaticle.typedb.driver.common.exception.TypeDBDriverException;

import static com.vaticle.typedb.driver.jni.typedb_driver.init_logging;

public abstract class NativeObject<T> {
static {
Loader.loadNativeLibraries();
init_logging();
}

public final T nativeObject;
Expand Down
8 changes: 0 additions & 8 deletions java/common/exception/TypeDBDriverException.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
*/
public class TypeDBDriverException extends RuntimeException {

@Nullable
private final com.vaticle.typedb.driver.jni.Error nativeError;

Comment on lines -31 to -33
Copy link
Member Author

Choose a reason for hiding this comment

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

We no longer store the native error and instead just take its message.

@Nullable
private final ErrorMessage errorMessage;

Expand All @@ -40,7 +37,6 @@ public class TypeDBDriverException extends RuntimeException {
public TypeDBDriverException(ErrorMessage error, Object... parameters) {
super(error.message(parameters));
assert !getMessage().contains("%s");
this.nativeError = null;
this.errorMessage = error;
}

Expand All @@ -49,7 +45,6 @@ public TypeDBDriverException(ErrorMessage error, Object... parameters) {
*/
public TypeDBDriverException(String message, Throwable cause) {
super(message, cause);
this.nativeError = null;
this.errorMessage = null;
}

Expand All @@ -59,8 +54,6 @@ public TypeDBDriverException(String message, Throwable cause) {
public TypeDBDriverException(RuntimeException error) {
super(error.getMessage());
assert !getMessage().contains("%s");
if (error.getCause() instanceof com.vaticle.typedb.driver.jni.Error) this.nativeError = (com.vaticle.typedb.driver.jni.Error)error.getCause();
else this.nativeError = null;
this.errorMessage = null;
}

Expand All @@ -70,7 +63,6 @@ public TypeDBDriverException(RuntimeException error) {
public TypeDBDriverException(com.vaticle.typedb.driver.jni.Error error) {
super(error.getMessage());
assert !getMessage().contains("%s");
this.nativeError = error;
this.errorMessage = null;
}

Expand Down
4 changes: 2 additions & 2 deletions rust/docs/concept/ConceptManager.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ transaction.concepts().get_relation_type(label).resolve()
----
pub fn get_schema_exceptions(
&self
) -> Result<impl Stream<Item = Result<SchemaException>>>
) -> Result<impl Stream<Item = Result<SchemaException>> + 'tx>
----

Retrieves a list of all schema exceptions for the current transaction.
Expand All @@ -345,7 +345,7 @@ Retrieves a list of all schema exceptions for the current transaction.
.Returns
[source,rust]
----
Result<impl Stream<Item = Result<SchemaException>>>
Result<impl Stream<Item = Result<SchemaException>> + 'tx>
----

[caption=""]
Expand Down
48 changes: 24 additions & 24 deletions rust/docs/data/Attribute.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ a| `value` a| `Value` a| The value which this Attribute instance holds.
[source,rust]
----
fn delete<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result>
----

Expand All @@ -52,12 +52,12 @@ BoxPromise<'tx, Result>

[source,rust]
----
fn get_has(
fn get_has<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
attribute_types: Vec<AttributeType>,
annotations: Vec<Annotation>
) -> Result<BoxStream<'_, Result<Attribute>>>
) -> Result<BoxStream<'tx, Result<Attribute>>>
----

<<#_trait_ThingAPI_method_get_has,Read more>>
Expand All @@ -66,19 +66,19 @@ fn get_has(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Attribute>>>
Result<BoxStream<'tx, Result<Attribute>>>
----

[#_struct_Attribute_method_get_owners]
==== get_owners

[source,rust]
----
fn get_owners(
fn get_owners<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
thing_type: Option<ThingType>
) -> Result<BoxStream<'_, Result<Thing>>>
) -> Result<BoxStream<'tx, Result<Thing>>>
----

<<#_trait_AttributeAPI_method_get_owners,Read more>>
Expand All @@ -87,18 +87,18 @@ fn get_owners(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Thing>>>
Result<BoxStream<'tx, Result<Thing>>>
----

[#_struct_Attribute_method_get_playing]
==== get_playing

[source,rust]
----
fn get_playing(
fn get_playing<'tx>(
&self,
transaction: &Transaction<'_>
) -> Result<BoxStream<'_, Result<RoleType>>>
transaction: &'tx Transaction<'_>
) -> Result<BoxStream<'tx, Result<RoleType>>>
----

<<#_trait_ThingAPI_method_get_playing,Read more>>
Expand All @@ -107,19 +107,19 @@ fn get_playing(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<RoleType>>>
Result<BoxStream<'tx, Result<RoleType>>>
----

[#_struct_Attribute_method_get_relations]
==== get_relations

[source,rust]
----
fn get_relations(
fn get_relations<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
role_types: Vec<RoleType>
) -> Result<BoxStream<'_, Result<Relation>>>
) -> Result<BoxStream<'tx, Result<Relation>>>
----

<<#_trait_ThingAPI_method_get_relations,Read more>>
Expand All @@ -128,7 +128,7 @@ fn get_relations(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Relation>>>
Result<BoxStream<'tx, Result<Relation>>>
----

[#_struct_Attribute_tymethod_iid]
Expand All @@ -154,8 +154,8 @@ fn iid(&self) -> &IID
[source,rust]
----
fn is_deleted<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result<bool>>
----

Expand Down Expand Up @@ -191,8 +191,8 @@ bool
[source,rust]
----
fn set_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand All @@ -212,8 +212,8 @@ BoxPromise<'tx, Result>
[source,rust]
----
fn unset_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand Down
40 changes: 20 additions & 20 deletions rust/docs/data/Entity.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ a| `type_` a| `EntityType` a| The type which this Entity belongs to
[source,rust]
----
fn delete<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result>
----

Expand All @@ -53,12 +53,12 @@ BoxPromise<'tx, Result>

[source,rust]
----
fn get_has(
fn get_has<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
attribute_types: Vec<AttributeType>,
annotations: Vec<Annotation>
) -> Result<BoxStream<'_, Result<Attribute>>>
) -> Result<BoxStream<'tx, Result<Attribute>>>
----

<<#_trait_ThingAPI_method_get_has,Read more>>
Expand All @@ -67,18 +67,18 @@ fn get_has(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Attribute>>>
Result<BoxStream<'tx, Result<Attribute>>>
----

[#_struct_Entity_method_get_playing]
==== get_playing

[source,rust]
----
fn get_playing(
fn get_playing<'tx>(
&self,
transaction: &Transaction<'_>
) -> Result<BoxStream<'_, Result<RoleType>>>
transaction: &'tx Transaction<'_>
) -> Result<BoxStream<'tx, Result<RoleType>>>
----

<<#_trait_ThingAPI_method_get_playing,Read more>>
Expand All @@ -87,19 +87,19 @@ fn get_playing(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<RoleType>>>
Result<BoxStream<'tx, Result<RoleType>>>
----

[#_struct_Entity_method_get_relations]
==== get_relations

[source,rust]
----
fn get_relations(
fn get_relations<'tx>(
&self,
transaction: &Transaction<'_>,
transaction: &'tx Transaction<'_>,
role_types: Vec<RoleType>
) -> Result<BoxStream<'_, Result<Relation>>>
) -> Result<BoxStream<'tx, Result<Relation>>>
----

<<#_trait_ThingAPI_method_get_relations,Read more>>
Expand All @@ -108,7 +108,7 @@ fn get_relations(
.Returns
[source,rust]
----
Result<BoxStream<'_, Result<Relation>>>
Result<BoxStream<'tx, Result<Relation>>>
----

[#_struct_Entity_tymethod_iid]
Expand All @@ -134,8 +134,8 @@ fn iid(&self) -> &IID
[source,rust]
----
fn is_deleted<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>
&self,
transaction: &'tx Transaction<'_>
) -> BoxPromise<'tx, Result<bool>>
----

Expand Down Expand Up @@ -171,8 +171,8 @@ bool
[source,rust]
----
fn set_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand All @@ -192,8 +192,8 @@ BoxPromise<'tx, Result>
[source,rust]
----
fn unset_has<'tx>(
&'tx self,
transaction: &'tx Transaction<'tx>,
&self,
transaction: &'tx Transaction<'_>,
attribute: Attribute
) -> BoxPromise<'tx, Result>
----
Expand Down
Loading