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

✨ Enable sharing dobjects & instances in the hub #69

Merged
merged 38 commits into from
Jul 29, 2022
Merged

Conversation

fredericenard
Copy link
Collaborator

@fredericenard fredericenard commented Jul 11, 2022

I created a push function for dobject.

We can either test it directly on the supabase prod workspace or create a proper dev branch that work with the supabase dev workspace.

If we choose to create a dev branch, we just need to specify an other file than https://lamin-site-assets.s3.amazonaws.com/connector.env when we create the hub.

For the moment I tested it on the prod supabase workspace.
It works only if I create a policy that allow every user to insert into the dobject table, without checking if the lnid belong really to them. Which is not what we want.

We have to discuss about this because it seems that the only way to get a session in order to do hub.postgrest.auth(session.access_token) is to do a sign_in.
Maybe I'm wrong, but if i not we could create an Auth object that will store session after sing_in, so that we can access it from other part of the project.

EDIT:

I finally understood that it was not a problem at all to do a sign_in, so i implemented the same logic as you to enable row level security.

I also manage to connect directly with the supabase dev, it was easier for me to test that everything appear in the frontend (lamin hub).

@github-actions
Copy link

github-actions bot commented Jul 11, 2022

@github-actions github-actions bot temporarily deployed to pull request July 11, 2022 14:24 Inactive
@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #69 (c7e7664) into main (6e7b29d) will decrease coverage by 19.02%.
The diff coverage is 67.26%.

@@             Coverage Diff             @@
##             main      #69       +/-   ##
===========================================
- Coverage   93.12%   74.10%   -19.03%     
===========================================
  Files          33       29        -4     
  Lines         524      421      -103     
===========================================
- Hits          488      312      -176     
- Misses         36      109       +73     
Impacted Files Coverage Δ
lamindb/do/_push.py 23.58% <23.58%> (ø)
lamindb/_check_versions.py 71.42% <71.42%> (ø)
lamindb/meta/_annotate.py 80.95% <80.95%> (ø)
lamindb/__init__.py 100.00% <100.00%> (+7.69%) ⬆️
lamindb/_logger.py 100.00% <100.00%> (+4.34%) ⬆️
lamindb/dev/__init__.py 100.00% <100.00%> (ø)
lamindb/dev/db/__init__.py 100.00% <100.00%> (ø)
lamindb/dev/db/_insert.py 100.00% <100.00%> (ø)
lamindb/dev/file/_file.py 90.00% <100.00%> (ø)
lamindb/dev/file/_h5ad.py 100.00% <100.00%> (ø)
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@github-actions github-actions bot temporarily deployed to pull request July 11, 2022 17:01 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 11, 2022 18:16 Inactive
from ..admin.db import get_engine


def push(dobject_id):
Copy link
Member

Choose a reason for hiding this comment

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

One of the checks that will fail is that you didn't write a docstring! 😅

@falexwolf
Copy link
Member

But I think this currently fails due to something unrelated to the linting pre-commit checks:
image

Could it be that there is still a bug in here? Happy to look through it with you!

@github-actions github-actions bot temporarily deployed to pull request July 14, 2022 17:40 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 14, 2022 18:15 Inactive
@falexwolf falexwolf changed the title create push function ✨ Enable to share dobjects in the hub Jul 15, 2022
@github-actions github-actions bot temporarily deployed to pull request July 15, 2022 10:32 Inactive
@falexwolf
Copy link
Member

I don't know why the test isn't picked up by coverage. It all looks correct to me. Some subtle aspect of the coverage setup.

@falexwolf falexwolf changed the title ✨ Enable to share dobjects in the hub ✨ Enable sharing dobjects & instances in the hub Jul 15, 2022
@github-actions github-actions bot temporarily deployed to pull request July 23, 2022 01:34 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 23, 2022 01:45 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 28, 2022 20:25 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 29, 2022 09:19 Inactive
@fredericenard fredericenard merged commit e71e797 into main Jul 29, 2022
@fredericenard fredericenard deleted the do-share branch July 29, 2022 09:29
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

2 participants