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

bump minimum panel version to 1.3.8 #284

Merged
merged 10 commits into from
Feb 2, 2024
Merged

bump minimum panel version to 1.3.8 #284

merged 10 commits into from
Feb 2, 2024

Conversation

pmeier
Copy link
Member

@pmeier pmeier commented Jan 19, 2024

No description provided.

@pmeier
Copy link
Member Author

pmeier commented Jan 19, 2024

As of 1.3.7rc2 there two issues that prevent adoption:

  • When trying to send a message, we are instantly met with an error before the message is even rendered. Here is the traceback:

    Traceback (most recent call last):
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/panel/chat/feed.py", line 486, in _prepare_response
        self._callback_state = CallbackState.RUNNING
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/contextlib.py", line 126, in __exit__
        next(self.gen)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/param/parameterized.py", line 307, in batch_call_watchers
        parameterized.param._batch_call_watchers()
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/param/parameterized.py", line 2506, in _batch_call_watchers
        self_._execute_watcher(watcher, events)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/param/parameterized.py", line 2468, in _execute_watcher
        watcher.fn(*args, **kwargs)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/panel/reactive.py", line 374, in _param_change
        self._apply_update(named_events, properties, model, ref)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/panel/reactive.py", line 302, in _apply_update
        self._update_model(events, msg, root, model, doc, comm)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/panel/layout/base.py", line 119, in _update_model
        super()._update_model(events, msg, root, model, doc, comm)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/panel/reactive.py", line 623, in _update_model
        super()._update_model(events, msg, root, model, doc, comm)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/panel/reactive.py", line 318, in _update_model
        model_val = getattr(model, attr)
      File "/home/philip/.conda/envs/ragna-dev/lib/python3.9/site-packages/bokeh/core/has_props.py", line 357, in __getattr__
        return super().__getattribute__(name)
    AttributeError: 'Card' object has no attribute '_callback_state'
    

    There is no mention of Ragna at all in the traceback so I guess this is coming from panel. I'll try to compile a minimal repro. Edit: See ChatInterface is broken on 1.3.7rc2 holoviz/panel#6241 Edit2: Fixed in 1.3.7rc3

  • Our auth workflow is broken. This happens because for some reason the cookie we set does not show up in pn.state.cookies. However, I can confirm that the cookie is indeed there through the browser console. Plus, through randomly refreshing the page and clicking login again I sometimes get past the authentication, i.e. panel picks up on the cookie. So far I haven't found pattern.

@philippjfr
Copy link

I was not able to track down any changes that could possibly have broken the auth flow. The panel/auth module was barely touched and the same is true of any server related code.

@pmeier
Copy link
Member Author

pmeier commented Jan 19, 2024

The panel/auth module was barely touched and the same is true of any server related code.

Unfortunately, we are rolling our custom auth flow. There is a refactor planned (#178), but for now we are stuck with it.

Here is the idea:

  1. When hitting / we check if we find a cookie. If that is not the case, we redirect to /auth

    if "auth_token" not in pn.state.cookies:
    return pn.pane.HTML("""<script>window.location.href = '/auth';</script>""")

  2. On /auth we let the user sign in and if that works, we set the cookie and redirect to /

    try:
    authed = await self.api_wrapper.auth(
    self.login_input.value, self.password_input.value
    )
    if authed:
    # Sets the cookie on the JS side
    self.custom_js = f""" document.cookie = "auth_token={self.api_wrapper.auth_token}; path:/"; """
    except Exception:
    authed = False
    if authed:
    # perform redirect
    pn.state.location.param.update(reload=True, pathname="/")

So far this works fine with 1.3.7. However when we hit / the second time, pn.state.cookies still does not have the cookie we have set. But I can verify through the browser console (document.cookie) that the cookie is there.

So it seems however panel gets the cookies from the browser is somehow broken. Maybe there was a cache introduced or the like?

@pmeier
Copy link
Member Author

pmeier commented Jan 22, 2024

Here is the issue that we need fixed for our auth flow: holoviz/panel#6252

@pmeier
Copy link
Member Author

pmeier commented Jan 22, 2024

Fix for the cookie issue is available in holoviz/panel#6254. However, 1.3.7 was already released and thus we now need to wait for 1.3.8 before we can update.

@pmeier pmeier changed the title bump minimum panel version to 1.3.7 bump minimum panel version to 1.3.8 Jan 22, 2024
@pmeier pmeier marked this pull request as ready for review January 24, 2024 21:32
@pmeier pmeier requested a review from nenb January 24, 2024 21:32
@pmeier pmeier linked an issue Jan 30, 2024 that may be closed by this pull request
@pmeier pmeier added this to the 0.2.0 milestone Feb 2, 2024
Copy link
Contributor

@nenb nenb left a comment

Choose a reason for hiding this comment

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

LGTM!

@pmeier pmeier merged commit c343820 into main Feb 2, 2024
12 checks passed
@pmeier pmeier deleted the bump-panel branch February 2, 2024 22:30
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.

Message placeholder does not work
3 participants