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/add md ds to connectors #55

Merged
merged 8 commits into from
Dec 11, 2024
Merged

Conversation

Cervangirard
Copy link
Contributor

@Cervangirard Cervangirard commented Dec 5, 2024

Add the possibility to store the datasources object inside a connectors :
Main changes are :

  • all functions to recreate datasources
  • datasource is an attribute of connectors obj
  • new class for nested connectors : nested_connectors
  • new logic to use the datasources from connect or recreate it if the user doesn't use connect

How to test :

  • use connect with a custom yaml file and use the datasources fct
  • use the connectors fct with custom connector fct and unse the datasources fct

Copy link
Contributor

@vladimirobucina vladimirobucina left a comment

Choose a reason for hiding this comment

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

Functionalities look ok. I just think we need to stay consistent when definin imports from packages in documentation. I had few other comments. Also don't know if we should enforce parameter checking in internal functions (at least checkmate). All in all it looks good.

R/connectors.R Outdated Show resolved Hide resolved
R/conts_datasources.R Outdated Show resolved Hide resolved
R/conts_datasources.R Outdated Show resolved Hide resolved
R/connectors.R Show resolved Hide resolved
Copy link

Code coverage

Name Coverage (%)
connector 76.23
R/cnt_generics.R 35.71
R/connect_utils.R 100.00
R/connect.R 95.43
R/connector.R 14.29
R/connectors.R 64.15
R/conts_datasources.R 97.85
R/dbi_backend_tools.R 66.67
R/dbi_methods.R 100.00
R/dbi.R 100.00
R/fs_backend_tools.R 66.67
R/fs_methods.R 100.00
R/fs_read.R 42.86
R/fs_write.R 66.67
R/fs.R 100.00
R/generic_backend.R 96.55
R/utils_fileext.R 100.00
R/utils_files.R 100.00
R/utils-docs.R 0.00
R/utils-roxygen.R 0.00

Copy link
Contributor

@vladimirobucina vladimirobucina left a comment

Choose a reason for hiding this comment

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

Now's good.

@Cervangirard Cervangirard merged commit 7c2c7cc into main Dec 11, 2024
12 of 14 checks passed
@Cervangirard Cervangirard deleted the feat/add_md_ds_to_connectors branch December 11, 2024 11:08
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