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

VDK-DuckDB: Introducing a new database plugin #2561

Merged
merged 20 commits into from
Aug 29, 2023
Merged

VDK-DuckDB: Introducing a new database plugin #2561

merged 20 commits into from
Aug 29, 2023

Conversation

Maximiliaan72
Copy link
Collaborator

@Maximiliaan72 Maximiliaan72 commented Aug 17, 2023

Why?

This plugin has been created to check the differences how databases are implemented in a VDK plugin.
The aim of this change is to add DuckDB to the list of database plugins.

What?

I made a new DuckDB-plugin based on the SQLite-plugin.
For making the DuckDB work I had to change several things.
For the Plugin I had to change: Module Imports, Configuration, Connection Factory Method, Ingester Method and Library Specific Code.
For the Ingestion I had to change: Import Statements, Configuration and connection, Target Path, Data Type Conversion, Query compatibility,
General naming and references, Connection Handling.
For the Configuration I had to change: Constants, Class Names, Method for Database File, Configuration Key, Descriptions.
For the Connection I had to change: Classes, Class documentation, File Name, Import Statement, Connection Method, Isolation Level.

How has this been tested?

I've made a test program that first relied on the 'auto create feature' of the plugin. But after several errors I realized
that probably the auto create function wasn't functioning as expected.
So now the test doesn't rely on this function and instead doesn't create the table in advance, disables auto create, checks if the table is missing
and makes sure the ingestion logic remains.

What type of change are you making?

This is a new feature.
Now the users can make use of the DuckDB database from the list of database plugins.

@Maximiliaan72 Maximiliaan72 changed the title vdk-DuckDB VDK-DuckDB: Introducing a new database plugin Aug 21, 2023
@Maximiliaan72 Maximiliaan72 enabled auto-merge (squash) August 23, 2023 13:49
Copy link
Contributor

@DeltaMichael DeltaMichael left a comment

Choose a reason for hiding this comment

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

Overall, the change looks ok. There are a few things you should do before merging:

  1. Rebase your branch to main. Right now you're submitting 22 commits. Ideally, it should be ~1.
git pull origin main --rebase
  1. Remove all the unnecessary files checked into version control, e.g. .db files
  2. Change your commit message to use our commit template https://github.com/vmware/versatile-data-kit/blob/main/support/git-commit-template.txt
  3. Add a description to the PR (usually, it's fine to just copy the commit message if you're using the template)
  4. Address all comments

Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Please remove also default_path.duckdb and temp.duckdb.

Normally, source code files are what you'd store in Git.
Auto-generated files, build artifacts or temporary files are not meant to be tracked in git. They are by-product of the code you write.

This enable us to keep the Git repository clean, organized, and efficient, adhering to best practices for source code management

@Maximiliaan72
Copy link
Collaborator Author

Overall, the change looks ok. There are a few things you should do before merging:

  1. Rebase your branch to main. Right now you're submitting 22 commits. Ideally, it should be ~1.
git pull origin main --rebase
  1. Remove all the unnecessary files checked into version control, e.g. .db files
  2. Change your commit message to use our commit template https://github.com/vmware/versatile-data-kit/blob/main/support/git-commit-template.txt
  3. Add a description to the PR (usually, it's fine to just copy the commit message if you're using the template)
  4. Address all comments

auto-merge was automatically disabled August 28, 2023 12:21

Pull request was closed

@Maximiliaan72 Maximiliaan72 reopened this Aug 28, 2023
Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the contribution!

Maximiliaan and others added 11 commits August 28, 2023 22:46
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Maximiliaan and others added 9 commits August 28, 2023 22:46
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
Created the basic plugin structure for DuckDB using the VDK CookieCutter with modifications.
@Maximiliaan72 Maximiliaan72 merged commit e1bb698 into main Aug 29, 2023
7 of 8 checks passed
@Maximiliaan72 Maximiliaan72 deleted the vdk-DuckDB branch August 29, 2023 07:30
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.

None yet

3 participants