-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat(ui): Make CaraML AI placeholder app generic #110
feat(ui): Make CaraML AI placeholder app generic #110
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
=======================================
Coverage 57.14% 57.14%
=======================================
Files 47 47
Lines 2443 2443
=======================================
Hits 1396 1396
Misses 851 851
Partials 196 196
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b071d63
to
7247035
Compare
7247035
to
b5afa85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for the quick review and the comments everyone! Merging this now! 🙏🏼 |
Context
This PR is a follow up from #109, which makes the CaraML AI placeholder app generic enough to support all forms of apps that need to be rendered as a page with an embedded Streamlit app by the MLP UI. It also consolidates all the configuration needed to render this page into a single location (the
Applications.PlaceholderPageConfig
struct).Essentially, while the CaraML AI placeholder app was previously hardcoded into existence in the MLP server/UI through various fragmented pieces like the
CaramlAIStreamlitHomepage
config or the hardcodedCaraMLAIPage.js
page's title, setting up new streamlit placeholder apps now simply requires specifying these in theApplications
config file that is read by the MLP server:Changes
Fundamentally, the changes to this PR are relatively straightforward, we simply remove the hardcoded streamlit app and conditionally generate React routes only after the list of apps have been retrieved from the
v2/applications
endpoint on the MLP API server. The ReactAppRoutes
are also made to intentionally not render until that happens (this is because React routes are rendered synchronously, meaning we can't add routes on-the-fly without re-rendering all the routes again).The
isLoaded
context value has also been added toApplicationsContext
as a useful flag to determine when the response from the applications endpoint has been returned.