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

Adds Databricks integration (#839) #841

Merged
merged 39 commits into from
Oct 3, 2024
Merged

Adds Databricks integration (#839) #841

merged 39 commits into from
Oct 3, 2024

Conversation

edgararuiz
Copy link
Contributor

Details

  • Adds board_databricks(), with support for prefix
  • Adds necessary methods for new backend: pin_list, pin_exists, pin_meta, pin_store, pin_versions, pin_fetch, pin_delete, pin_version_delete, board_deparse and required_pkgs
  • Passes test_api_basic(), test_api_versioning(), test_api_meta().
  • Adds necessary tests

Test Results

I am emulating how the other boards implement required_pkgs, but I still get Warnings. Not sure how to address, or if I should address it in this PR.

==> Testing R file using 'testthat'Loading pins
[ FAIL 0 | WARN 2 | SKIP 0 | PASS 38 ]Guessing `type = 'rds'`
[ FAIL 0 | WARN 2 | SKIP 0 | PASS 61 ]

── Warning (test-board-databricks.R:10:1): can find board required pkgs ────────
Adding new snapshot:
Code
  required_pkgs(board)
Output
  [1] "httr2"

── Warning (test-board-databricks.R:11:1): can find board required pkgs ────────
Adding new snapshot:
Code
  required_pkgs(board)
Output
  [1] "httr2"
[ FAIL 0 | WARN 2 | SKIP 0 | PASS 61 ]

Test complete

(Fixes #839)

Copy link
Member

@juliasilge juliasilge left a comment

Choose a reason for hiding this comment

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

I have a request in to CloudOps to get access to Databricks so I can run the tests and such.

R/board_databricks.R Outdated Show resolved Hide resolved
R/board_databricks.R Outdated Show resolved Hide resolved
R/board_databricks.R Outdated Show resolved Hide resolved
Copy link
Member

@juliasilge juliasilge left a comment

Choose a reason for hiding this comment

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

Thank you so much for all this work @edgararuiz! 🙌

I was able to get access to Databricks and a Volume to test this myself, and all appears to be working well. I also added creds to CI so we can run the tests as part of the check-boards job, which is working well.

@juliasilge juliasilge merged commit 770ef00 into main Oct 3, 2024
14 checks passed
@juliasilge juliasilge deleted the databricks branch October 3, 2024 01:16
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Save Pins to Databricks
2 participants