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

Cookie management via solara #773

Open
JovanVeljanoski opened this issue Sep 5, 2024 · 11 comments
Open

Cookie management via solara #773

JovanVeljanoski opened this issue Sep 5, 2024 · 11 comments

Comments

@JovanVeljanoski
Copy link
Collaborator

I saw that in the documentation here on how to read cookies via solara.

But is there a way to set cookies also? Changing the reactive variable solara.lab.cookies is not communicated back to the browser.

Thanks.

@maartenbreddels
Copy link
Contributor

Hi Jovan,

Indeed, as mentioned in the docs, we do not support writing cookies yet (again, details to get that right).
I've made an example here:
https://py.cafe/maartenbreddels/solara-cookie-storage
Which does not fully work the same on pycafe (yet) as it does on a real server. Also, it highlights an issue (the on_kernel_start is useless, and we do this workaround for it).
I'll open an issue for that workaround, since it would have made the example much nicer.
Also, I'm not super happy about this code right now, as it doesn't look really nicely reusable.

The biggest difference to #772 is that (ignoring the issue on pycafe right now, which should be fixed) we can render with the initial value directly, because we also get the cookies before we render.

cheers,

Maarten

@JovanVeljanoski
Copy link
Collaborator Author

Hey,

Thank you for the swift response. This should do for now.

Is there a plan for supporting this in a more mature / integrated way in the (near) future?

Thank you

@maartenbreddels
Copy link
Contributor

I do want to fix #774, to make this at least 80% less messy.

However, the issue is that if we want to make this supported, we not only have to support writing to cookies, but we need support for setting (with an expiry date), and clearing. So if we find a nice API/method for that, then we could indeed think about supporting it.

The solution I gave now (minus the ugly hacks) gives you a bit more control for now.

Short answer: yes, we'd like to have it, but it is not our biggest priority (although #774 is much higher!)

@JovanVeljanoski
Copy link
Collaborator Author

Ok that sounds pretty good already!

If may ask a follow up question.., in your example above, what purpose does on_value have in

def CookieStore(key: str, value: str, on_value: Callable[[str], None] = None, debug: bool=False):
...

I've been experimenting with it.. but it is not obvious to me what is happening (without good understanding of the underlying vue code).

Thanks!

@JovanVeljanoski
Copy link
Collaborator Author

I thought I finally understood how this works.. but then I found out I did not :).

Is it possible to get some advice on the usage here?
Link to a non-completely working example on py.cafe.

(I don't expect this to work on py.cafe due to the issue @maartenbreddels highlighted above, but make is easier to share a more involved example).

The issue explained: I don't know where exactly in this process the cookie is read, on start the solara.lab.cookies.value appears to be None (but the cookie exists, in my local example), but when connected to a CookieStore component, the cookie is read correctly.

In my current multi-page example I can not get the setting of the cookie to work at all, at best I over-write original cookie value.

I can get a simpler single page example to work correctly, I can set and get cookies. But this multi-page example has me puzzled for a longer time.

I would like to know if I am pushing solara beyond its (current) capabilities, or am I doing something wrong (more likely) and I've not understood the logic properly.

Any advice would be appreciated.
Thank you.

@maartenbreddels
Copy link
Contributor

Could you describe what the intent of the app is? If I login, I get redirect to callback and back again, but I don't know what it should do. At what place should the cookie be set?
Also note that init_cookie_value is only called when the module is imported (not during app execution).

However, maybe this simple change will make the app do what you want:

    # update_state(my_value=my_value)
    def update():
        update_state(my_value=my_value)
        router.push('/')
    solara.use_effect(update, [])

You were doing a router.push directly in the render body, and also doing the update_state directly in the body. Although the update_state shouldn't be problematic (not 100% if you found a bug or not), this is a better approach. You let the render phase finish, and the use effect executes afterwards.

I do feel like if we were to add support for cookies, it would avoid a lot of boilerplate in this app

@JovanVeljanoski
Copy link
Collaborator Author

JovanVeljanoski commented Sep 9, 2024

Hi,

Good question, maybe I should have mentioned that.

So the intent is to interface with a 3rd party authentication system (oauth like). So the login function would call the external api that will ask for credentials. If successful, it will redirect to the "callback" page. At that point, I want to store the relevant cookie.

When the user appears on the home page (the "/" route), i want to already check for the presence of a valid cookie. If a valid cookie is found, the whole login step would be skipped (not implemented in the example, i got stuck on the gettin/setting cookies). I hope this makes sense.

But indeed, official support for cookies would be great, also for various other needs (like storing user settings, preferences etc..).

Thanks!

P.S.:

Thanks for the example/suggestion. I understand the intent - but the new value of the cookie is not written in the browser. Perhaps something in the vue component needs to be updated, but this is where my knowledge is limited. I might try with some LLM help later.

@maartenbreddels
Copy link
Contributor

maartenbreddels commented Sep 11, 2024

Still not working on py.cafe, but this is already works much better:

example

There were multiple things going wrong, much of those are explained in the comment. The main difficulty was that the CookieStore was used for both initialization and writing. Now that we can initialize (with the workaround) it becomes already simpler. Second issue was that even if the value was set (of the CookieStore), the message still needs to arrive at the browser, and the cookie be written. Only after the cookie is written should the page navigation happen.

@maartenbreddels
Copy link
Contributor

Also note that I'm using our new recommended pattern for state management using plain Python objects with reactive attributes, see #781

@JovanVeljanoski
Copy link
Collaborator Author

JovanVeljanoski commented Sep 11, 2024

Hi,

Thank you for the detailed example - cleared up many things and subtleties (like the need to wrap components in a Column etc..). Thank you!

For anything else following: the link does not work outright, but just change the "localhost:3001" to "py.cafe". Maybe @maartenbreddels you can update it.

I am very happy with all of these explanations, thank you. I am happy for this issue to be closed, or to be kept open for others to find it or until the cookies support is officially supported - I will leave it up to you.

@maartenbreddels
Copy link
Contributor

Ah thanks, yes, I have cookies on pycafe working locally, so that's where I tested this.

Lets keep this open!

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

No branches or pull requests

2 participants