feat(rust): add public abstract API and dummy driver implementation #1725

alexandreyc · 2024-04-16T12:46:51Z

Hey!

As discussed in #1723, here is the first PR of the new complete Rust implementation.

It includes:

Traits and structs definitions for the public abstract API
An implementation of a dummy driver for testing purposes (no test currently because we need the driver manager and exporter to have relevant tests)
Constants from adbc.h

I put the code in /rust2 but I can move it to /rust if you prefer.

CC @mbrobbel @paleolimbot @zeroshade @lidavidm

paleolimbot

Awesome! I'm happy to take a few passes on the non-Rust specific stuff 🙂

From CI I see:

Check for unapproved licenses............................................Failed
- hook id: apache-rat
- exit code: 1

~/work/arrow-adbc/arrow-adbc ~/work/arrow-adbc/arrow-adbc
NOT APPROVED: rust2/Cargo.toml (adbc/rust2/Cargo.toml): false
NOT APPROVED: rust2/README.md (adbc/rust2/README.md): false
NOT APPROVED: rust2/core/Cargo.toml (adbc/rust2/core/Cargo.toml): false
NOT APPROVED: rust2/core/src/error.rs (adbc/rust2/core/src/error.rs): false
NOT APPROVED: rust2/core/src/ffi/constants.rs (adbc/rust2/core/src/ffi/constants.rs): false
NOT APPROVED: rust2/core/src/ffi/mod.rs (adbc/rust2/core/src/ffi/mod.rs): false
NOT APPROVED: rust2/core/src/ffi/types.rs (adbc/rust2/core/src/ffi/types.rs): false
NOT APPROVED: rust2/core/src/lib.rs (adbc/rust2/core/src/lib.rs): false
NOT APPROVED: rust2/core/src/options.rs (adbc/rust2/core/src/options.rs): false
NOT APPROVED: rust2/core/src/schemas.rs (adbc/rust2/core/src/schemas.rs): false
NOT APPROVED: rust2/drivers/dummy/Cargo.toml (adbc/rust2/drivers/dummy/Cargo.toml): false
NOT APPROVED: rust2/drivers/dummy/src/lib.rs (adbc/rust2/drivers/dummy/src/lib.rs): false
12 unapproved licences. Check rat report: rat.txt

...in order to get this to go away you'll have to add the Apache license header to your files:

arrow-adbc/rust/src/info.rs

Lines 1 to 16 in aa04aad

    
           // Licensed to the Apache Software Foundation (ASF) under one 
        
           // or more contributor license agreements.  See the NOTICE file 
        
           // distributed with this work for additional information 
        
           // regarding copyright ownership.  The ASF licenses this file 
        
           // to you under the Apache License, Version 2.0 (the 
        
           // "License"); you may not use this file except in compliance 
        
           // with the License.  You may obtain a copy of the License at 
        
           // 
        
           //   http://www.apache.org/licenses/LICENSE-2.0 
        
           // 
        
           // Unless required by applicable law or agreed to in writing, 
        
           // software distributed under the License is distributed on an 
        
           // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
        
           // KIND, either express or implied.  See the License for the 
        
           // specific language governing permissions and limitations 
        
           // under the License.

It will probably also help reviewers to see the results of a CI run. Would it be appropriate to modify https://github.com/apache/arrow-adbc/blob/aa04aadccd319e6fa3abb07154fa8d87b58d5c21/.github/workflows/rust.yml to also run for this directory?

alexandreyc · 2024-04-16T14:26:43Z

Thanks for taking time to review!

I've added the licence header to all files. The CI is now green.

I'm not very familiar with Github Actions so I need a bit more time to read doc before integrating the new folder to CI.

aljazerzen

I've left a few comments about what I would expect the API to be from the "Rust developer" standpoint. We need to find a balance between making the Rust API similar to the C API and making it "ideomatic". With this I mean that it would be hard to misuse it and that it would be well organized, so it is easy to discover and use.

rust2/core/src/lib.rs

paleolimbot · 2024-04-16T17:53:40Z

I'm not very familiar with Github Actions so I need a bit more time to read doc before integrating the new folder to CI.

The final CI may be a bit different, but for the purposes of getting the review started, perhaps just modify:

arrow-adbc/.github/workflows/rust.yml

Line 41 in aa04aad

working-directory: rust

...such that it reads working-directory: rust2

mbrobbel

Thanks, this looks great.

I put the code in /rust2 but I can move it to /rust if you prefer.

+1 to just move this to /rust.

I'm not very familiar with Github Actions so I need a bit more time to read doc before integrating the new folder to CI.

I can help figure out CI in follow-up PRs.

rust2/Cargo.toml

rust2/core/Cargo.toml

rust2/core/src/error.rs

rust2/core/src/options.rs

…f ref

rust2/core/src/options.rs

.github/workflows/rust.yml

rust2/drivers/dummy/src/lib.rs

mbrobbel

My final suggestion/nit: move this to rust (instead of rust2).

alexandreyc · 2024-04-18T13:38:46Z

Should be ready to merge now!

Next PR: full FFI bindings

@mbrobbel If you have time, I think that you can work on the CI in parallel (there shouldn't be any conflict)

lidavidm · 2024-04-18T23:37:38Z

I'll merge this in a day or two if there's no further comments. Thanks @alexandreyc!

lidavidm · 2024-04-22T01:39:52Z

Augh, I forgot to edit out the pings from the commit message...

mbrobbel · 2024-04-23T11:18:11Z

@mbrobbel If you have time, I think that you can work on the CI in parallel (there shouldn't be any conflict)

#1749

@mbrobbel

…pache#1725) Hey! As discussed in apache#1723, here is the first PR of the new complete Rust implementation. It includes: - Traits and structs definitions for the public abstract API - An implementation of a dummy driver for testing purposes (no test currently because we need the driver manager and exporter to have relevant tests) - Constants from `adbc.h` I put the code in `/rust2` but I can move it to `/rust` if you prefer. CC @mbrobbel @paleolimbot @zeroshade @lidavidm

@mbrobbel

…pache#1725) Hey! As discussed in apache#1723, here is the first PR of the new complete Rust implementation. It includes: - Traits and structs definitions for the public abstract API - An implementation of a dummy driver for testing purposes (no test currently because we need the driver manager and exporter to have relevant tests) - Constants from `adbc.h` I put the code in `/rust2` but I can move it to `/rust` if you prefer. CC @mbrobbel @paleolimbot @zeroshade @lidavidm

@mbrobbel

…pache#1725) Hey! As discussed in apache#1723, here is the first PR of the new complete Rust implementation. It includes: - Traits and structs definitions for the public abstract API - An implementation of a dummy driver for testing purposes (no test currently because we need the driver manager and exporter to have relevant tests) - Constants from `adbc.h` I put the code in `/rust2` but I can move it to `/rust` if you prefer. CC @mbrobbel @paleolimbot @zeroshade @lidavidm

As discussed in #1725 (comment), we can now use the `arrow-*` subcrates. This also bumps these `arrow` crates to the latest release. This is a breaking change for the `adbc_core` crate.

Add public abstract API and dummy driver implementation

1af2e5b

github-actions bot added this to the ADBC Libraries 1.0.0 milestone Apr 16, 2024

alexandreyc mentioned this pull request Apr 16, 2024

feat(rust): add driver manager and ability to expose Rust driver as C driver #1496

Closed

paleolimbot reviewed Apr 16, 2024

View reviewed changes

alexandreyc added 2 commits April 16, 2024 16:13

Add license header to all files

0951bdf

Trim trailing whitespace

454b80c

aljazerzen reviewed Apr 16, 2024

View reviewed changes

rust2/core/src/lib.rs Outdated Show resolved Hide resolved

rust2/core/src/lib.rs Outdated Show resolved Hide resolved

Update Statement::execute_update to return Option

7eda7f6

Update CI to run on the new directory

99a8484

alexandreyc requested review from lidavidm and zeroshade as code owners April 16, 2024 18:02

mbrobbel reviewed Apr 16, 2024

View reviewed changes

alexandreyc added 8 commits April 16, 2024 20:43

Update to inline variables in format strings

fa8c14c

Update to pass AsRef<[u8]> to read_partition

d0d0eae

Update Error::with_message_and_status to take Into<String>

c47ce0b

Add non_exhaustive attribute to relevant enums

a29fff4

Update ADBC_VERSION_* constants to c_int

6d7ba2c

Update Error.sqlstate to c_char array

28a840f

Update get_into to take an HashSet

791a11d

Update new_database, new_connection and new_statement to take mut sel…

2e80414

…f ref

aljazerzen reviewed Apr 17, 2024

View reviewed changes

rust2/core/src/options.rs Show resolved Hide resolved

paleolimbot reviewed Apr 17, 2024

View reviewed changes

.github/workflows/rust.yml Show resolved Hide resolved

alexandreyc added 2 commits April 17, 2024 15:55

Add todo to revisit in follow-up PRs

4e25894

Update some methods to take ref mut self

a6a0fd7

alexandreyc force-pushed the rust-part1 branch from f2ba8e2 to a6a0fd7 Compare April 18, 2024 11:03

mbrobbel reviewed Apr 18, 2024

View reviewed changes

rust2/drivers/dummy/src/lib.rs Outdated Show resolved Hide resolved

rust2/drivers/dummy/src/lib.rs Outdated Show resolved Hide resolved

rust2/drivers/dummy/src/lib.rs Show resolved Hide resolved

Update opts argument to be IntoIterator

38502ef

mbrobbel approved these changes Apr 18, 2024

View reviewed changes

Move code from /rust2 to /rust

79773cf

alexandreyc requested a review from wjones127 as a code owner April 18, 2024 13:35

alexandreyc mentioned this pull request Apr 18, 2024

Extract ffi_stream and to_ffi/from_ffi to a separate crate? apache/arrow-rs#5663

Closed

alexandreyc mentioned this pull request Apr 19, 2024

Move ffi stream and utils from arrow to arrow-array apache/arrow-rs#5670

Merged

lidavidm merged commit 922e69b into apache:main Apr 22, 2024

aljazerzen mentioned this pull request May 7, 2024

Future of this crate and ADBC aljazerzen/connector_arrow#28

Open

mbrobbel mentioned this pull request Oct 1, 2024

chore(rust)!: use arrow-* subcrates and bump to 53.1.0 #2208

Merged

mbrobbel mentioned this pull request Oct 11, 2024

feat(rust/driver/snowflake): add adbc_snowflake crate with Go driver wrapper #2207

Merged

4 tasks

	// Licensed to the Apache Software Foundation (ASF) under one
	// or more contributor license agreements. See the NOTICE file
	// distributed with this work for additional information
	// regarding copyright ownership. The ASF licenses this file
	// to you under the Apache License, Version 2.0 (the
	// "License"); you may not use this file except in compliance
	// with the License. You may obtain a copy of the License at
	//
	// http://www.apache.org/licenses/LICENSE-2.0
	//
	// Unless required by applicable law or agreed to in writing,
	// software distributed under the License is distributed on an
	// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
	// KIND, either express or implied. See the License for the
	// specific language governing permissions and limitations
	// under the License.

feat(rust): add public abstract API and dummy driver implementation #1725

feat(rust): add public abstract API and dummy driver implementation #1725

Uh oh!

Conversation

alexandreyc commented Apr 16, 2024

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

alexandreyc commented Apr 16, 2024

Uh oh!

aljazerzen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paleolimbot commented Apr 16, 2024

Uh oh!

mbrobbel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbrobbel left a comment

Choose a reason for hiding this comment

Uh oh!

alexandreyc commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidavidm commented Apr 18, 2024

Uh oh!

lidavidm commented Apr 22, 2024

Uh oh!

mbrobbel commented Apr 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alexandreyc commented Apr 18, 2024 •

edited

Loading