Skip to content

Explicitly mention that changes to config file while server is live requires restarting the server #548

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

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

achieveordie
Copy link
Contributor

📚 Context

Fixes #535

🧠 Description of Changes

  • Add a callout in /library/advanced-features/configuration.md mentioning the need to restart the server if changes to config.toml are made while the app is running.

Revised:
new

Current:
current

💥 Impact

Size:

  • Small
  • Not small

🌐 References

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@achieveordie
Copy link
Contributor Author

@snehankekre I hope this looks good.

PS: I couldn't find which npm version was required for the build, on 8.19.2, it gave me a possibly critical warning as:

npm WARN deprecated [email protected]: core-js-pure@<3.23.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Some versions have web compatibility issues. Please, upgrade your dependencies to the actual version of core-js-pure

I thought I should mention it but since I'm fairly new to npm I'm not sure if it really is troublesome or not.

@snehankekre snehankekre self-requested a review December 9, 2022 06:16
Copy link
Contributor

@snehankekre snehankekre left a comment

Choose a reason for hiding this comment

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

The callout looks good to me. Thank you for your contribution, @achieveordie!!

@snehankekre
Copy link
Contributor

Thanks for pointing out the issue in npm versions. I'll create a follow up PR to clarify the build instruction in our README.

Personally, I've been using npm 8.11.0 by following the installation steps in the Streamlit library repo. I'll post those instructions to this repo.

@snehankekre snehankekre merged commit 1b4cecd into streamlit:main Dec 12, 2022
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.

Explicity mentioning that (local?) configuration changes requires a restart of server
2 participants