Skip to content

dev/upgrade streamlit to 1.30.0#678

Merged
whitphx merged 6 commits intomainfrom
dev/upgrade-streamlit-1.30.0
Jan 20, 2024
Merged

dev/upgrade streamlit to 1.30.0#678
whitphx merged 6 commits intomainfrom
dev/upgrade-streamlit-1.30.0

Conversation

@whitphx
Copy link
Copy Markdown
Owner

@whitphx whitphx commented Jan 19, 2024

  • Upgrade Streamlit to 1.30.0
  • Fix hostConfigResponse in Kernel
  • Update hostConfigResponse callers

* Note that Streamlit's iframe messaging referred to here is different from the iframe messaging mechanism implemented for the iframe embedded on stlite sharing.
*/
allowedOriginsResp?: IAllowedMessageOriginsResponse;
hostConfigResponse?: IHostConfigResponse;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Catch up with the change in streamlit/streamlit#7342

archives,
requirements: options.requirements || [],
allowedOriginsResp: options.allowedOriginsResp,
hostConfigResponse: options.hostConfigResponse,
Copy link
Copy Markdown
Contributor

@lukasmasuch lukasmasuch Jan 19, 2024

Choose a reason for hiding this comment

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

nit: I think it is better to call this just hostConfig here when treating it as an option that is exposed to the user. We will also soon add some official documentation about host config which explains what options exist. Calling it hostConfigResponse could make this slightly more confusing / technical.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Agreed! Thanks :D

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Did it.

tbh, I haven't understood well about how the HostConfig API could be used by users and why it is separated from the existing config mechanism (config.toml or the command line arguments), so also don't know what's the best signature of this API on stlite (should it be integrated into the streamlitConfig field in the case of stlite, for example?).
Actually, currently this field is only used by the VSCode extension package internally so we can change the signature in the future anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so high level: host config is targeting host platforms while the app config (config.toml) is targeting app developers. But some aspects overlap. In a normal Streamlit setup, these mechanisms work quite differently (because we need to ensure tamper-proofness for host config). For stlite, the separation doesn't matter a lot. It probably could be merged into one. But I think it is better to keep this as two options since it later can be better connected to the dedicated pages on our documentation. Once we have something out on our docs, it might be a bit more understandable.

@whitphx
Copy link
Copy Markdown
Owner Author

whitphx commented Jan 20, 2024

Oh the test started to fail from today due to this date-dependent deprecation

https://github.com/streamlit/streamlit/blob/a2e694b56c0f829b7b09a41a497a1df0c9a734ae/lib/streamlit/config.py#L567

@whitphx
Copy link
Copy Markdown
Owner Author

whitphx commented Jan 20, 2024

@whitphx
Copy link
Copy Markdown
Owner Author

whitphx commented Jan 20, 2024

I'm gonna merge this.
Plz leave any comment if you have even after that!

@whitphx whitphx merged commit 199c796 into main Jan 20, 2024
@whitphx whitphx deleted the dev/upgrade-streamlit-1.30.0 branch January 20, 2024 11:32
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.

2 participants