search

laminlabs / lamindb-setup

Setup & configure LaminDB.
Apache License 2.0
4 stars 2 forks source link

🐛 Make `set_managed_storage()` safer #822

Closed Koncopd closed 4 weeks ago

Koncopd commented 4 weeks ago

https://laminlabs.slack.com/archives/C07DB677JF6/p1723623568991179?thread_ts=1723618378.437019&cid=C07DB677JF6

github-actions[bot] commented 4 weeks ago

🚀 Deployed on https://66bf0cf8ac8fdbdcc1807926--lamindb-setup-htry.netlify.app

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 83.05%. Comparing base (adc9e1c) to head (1625ddc). Report is 1 commits behind head on main.

Files Patch % Lines
lamindb_setup/_set_managed_storage.py 50.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #822 +/- ## ========================================== - Coverage 83.42% 83.05% -0.38% ========================================== Files 42 42 Lines 3234 3240 +6 ========================================== - Hits 2698 2691 -7 - Misses 536 549 +13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

falexwolf commented 4 weeks ago

This is good, but please take the time to write a test for it and add the comments. It's very fundamental.

We'll also still need to update RLS, right? It's not a security concern, only an integrity concern, hence less urgent.

Koncopd commented 4 weeks ago

Not sure what kind of test can be added here, i am not really sure it is a good idea to add something failing, touching hub every time, than deleting the record to the CI.

falexwolf commented 4 weeks ago

It's totally OK to add some failing and touch the hub all the time. It can deal with millions of requests.

Koncopd commented 4 weeks ago

Maybe even in hub-local?

Koncopd commented 4 weeks ago

I will try to add some testing here.

Koncopd commented 4 weeks ago

I am not worried about the number of requests, rather about adding some garbage to the hub, and it will happen every time something incorrect happens to this flow (in future code edits etc).