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.

Sign up for GitHub

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

Handle correctly not found errors in admin and chooser parts. #1060

Merged
merged 7 commits into from
Apr 2, 2024
Merged

Handle correctly not found errors in admin and chooser parts. #1060

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
86e4947
*handle correctly not found errors in `admin` part.
dsuhinin Mar 28, 2024
f522fa1
*adopt `chooser` part to use `not-found` page.
dsuhinin Mar 28, 2024
0c54845
*fox integration tests. integrate `not-found` page into `chooser`.
dsuhinin Mar 28, 2024
caf22ef
Merge branch 'main' into dsuhinin/error-handling
dsuhinin Apr 2, 2024
ded0c37
*PR remarks.
dsuhinin Apr 2, 2024
c2c6ed9
Merge remote-tracking branch 'origin/dsuhinin/error-handling' into ds…
dsuhinin Apr 2, 2024
4b0ab8b
*linter remarks.
dsuhinin Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/ui/admin/embed/layouts/main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>FastTrackML</title>
<link rel="stylesheet" href="/static/aim/assets/icomoon/icomoonIcons.css">
<link rel="stylesheet" href="/static/chooser/css/simple.min.css">
<link rel="stylesheet" href="/static/chooser/static/css/simple.min.css">
<link rel="stylesheet" href="/admin/static/css/namespaces.css">
<link rel="icon" type="image/x-icon" href="/static/chooser/favicon.ico">
<link rel="icon" type="image/x-icon" href="/static/chooser/static/favicon.ico">
Copy link
Contributor

Choose a reason for hiding this comment

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

/static/chooser/static seems like one-too-many static directories.

Copy link
Collaborator Author

@dsuhinin dsuhinin Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

unfortunately this is how it should be handled. we have new routes /chooser/namespaces so we can't for example start with just a chooser for our static data because it will be a collision. I tried different ways and current one is the most working.

Copy link
Contributor

@suprjinx suprjinx Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Well at least the sources do not have static/../static path, but it seems like there'd be a way to avoid this url path.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let me try to play but Im not fully sure. we we can skip first static from /static/chooser/static/, then it means that we have to mount routing to just /chooser which won't work. means if we call /chooser/namespaces, it wont work, because /chooser will catch this call which is logical.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, a bit reorganised everything.

<script type="text/javascript" language="javascript" src="/admin/static/js/jquery-3.7.0.js"></script>
<script type="text/javascript" language="javascript" src="/admin/static/js/namespaces.js"></script>
</head>
Expand All @@ -17,8 +17,8 @@
<header>
<a href="/">
<picture>
<source media="(prefers-color-scheme: dark)" srcset="/static/chooser/media/logo-dark.svg">
<img class="logo" src="/static/chooser/media/logo-light.svg" alt="FastTrackML logo">
<source media="(prefers-color-scheme: dark)" srcset="/static/chooser/static/media/logo-dark.svg">
<img class="logo" src="/static/chooser/static/media/logo-light.svg" alt="FastTrackML logo">
</picture>
</a>
<p>A <i>very fast</i> experiment tracker</p>
Expand Down
6 changes: 1 addition & 5 deletions pkg/ui/admin/middleware/authorization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ func NewAdminUserMiddleware(userPermissions *models.UserPermissions) fiber.Handl
strings.Replace(ctx.Get(fiber.HeaderAuthorization), "Basic ", "", 1),
)
if authToken == nil || !authToken.HasAdminAccess() {
return ctx.Status(
http.StatusNotFound,
).SendString(
"unable to find requested resource",
)
return ctx.Redirect("/errors/not-found", http.StatusMovedPermanently)
}

return ctx.Next()
Expand Down
10 changes: 10 additions & 0 deletions pkg/ui/chooser/controller/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package controller

import (
"github.com/gofiber/fiber/v2"
)

// NotFoundError renders Not Found error page.
func (c Controller) NotFoundError(ctx *fiber.Ctx) error {
return ctx.Render("errors/not-found", fiber.Map{})
}
4 changes: 2 additions & 2 deletions pkg/ui/chooser/controller/namespaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ func (c Controller) GetNamespaces(ctx *fiber.Ctx) error {
if err != nil {
return err
}
return ctx.Render("index", fiber.Map{
return ctx.Render("namespaces/index", fiber.Map{
"IsAdmin": isAdmin,
"Namespaces": namespaces,
"CurrentNamespace": ns,
})
}

// ListNamespaces handles `GET /namespaces/list` endpoint.
// ListNamespaces handles `GET /namespaces` endpoint.
func (c Controller) ListNamespaces(ctx *fiber.Ctx) error {
namespaces, _, err := c.namespaceService.ListNamespaces(ctx.Context())
if err != nil {
Expand Down
126 changes: 0 additions & 126 deletions pkg/ui/chooser/embed/index.html
View file
Open in desktop

This file was deleted.

82 changes: 82 additions & 0 deletions pkg/ui/chooser/embed/layouts/main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>FastTrackML</title>
<link rel="stylesheet" href="/static/chooser/static/css/simple.min.css">
<link rel="stylesheet" href="/static/aim/assets/icomoon/icomoonIcons.css">
<link rel="icon" type="image/x-icon" href="/static/chooser/static/favicon.ico">
<style>
:root {
--accent: #00a4e0;
}

main {
text-align: center;
}

.ui {
display: grid;
grid-template-columns: 1fr 1fr;
column-gap: 2rem;
}

.logo {
height: 150px;
margin-top: -25px;
margin-bottom: -50px;
}

#namespaces-list-container {
display: flex;
align-items: center;
overflow:auto;
border: 1px solid var(--border);
border-radius: 5px;
padding: 1rem;
}

#namespaces-list {
flex-direction: column;
align-items: first baseline;
text-align: left;
list-style-type: none;
margin-left: auto;
margin-right: auto;
}

#current-namespace {
margin-left: -1.3em;
position: relative;
}

#current-icon {
position: relative;
left: -1.5em;
}

</style>
</head>

<body>
<header>
<picture>
<source media="(prefers-color-scheme: dark)" srcset="/static/chooser/static/media/logo-dark.svg">
<img class="logo" src="/static/chooser/static/media/logo-light.svg" alt="FastTrackML logo">
</picture>
<p>A <i>very fast</i> experiment tracker</p>
</header>

<!-- We load the UI with JS to avoid issues when an URL with embedded credentials is used -->
<main>
{{ embed }}
</main>

<footer>
<p>Brought to you by <a href="https://opensource.gresearch.co.uk" target="_blank">G-Research Open-Source</a>.
</p>
</footer>
</body>

</html>
43 changes: 43 additions & 0 deletions pkg/ui/chooser/embed/namespaces/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<p>Which UI do you want to use?</p>
<div class="ui">
<article>
<a href="#" onclick="window.location = window.location.origin + window.location.pathname + 'mlflow/'">
<button>Classic</button>
</a>
<p>This is the classic MLFlow UI, albeit fast and responsive.</p>
</article>
<article>
<a href="#" onclick="window.location = window.location.origin + window.location.pathname + 'aim/'">
<button>Modern</button>
</a>
<p>This is the modern Aim UI, much faster than MLFlow.</p>
</article>
</div>
<p>Selected namespace:
<b>
<u>{{.CurrentNamespace.DisplayName}}</u>
</b>
</p>

<div id="namespaces-list-container">
<ul id="namespaces-list">
{{ range .Namespaces }}
{{ if ne .DisplayName $.CurrentNamespace.DisplayName }}
<li>
<a href="#" onclick="window.location = window.location.origin + {{if eq .Code "default"}}'/'{{else}}'/ns/{{.Code}}/'{{end}}">{{.DisplayName}}</a>
</li>
{{ else }}
<li class="selected-namespace">
<i id="current-icon" class="Icon__container icon-long-arrow-right"></i>
<b id="current-namespace">{{$.CurrentNamespace.DisplayName}}</b>
</li>
{{ end }}
{{ end }}
</ul>
</div>

{{ if .IsAdmin }}
<p>
<a href="#" onclick="window.location = window.location.origin + '/admin/namespaces/'">Manage namespaces</a>
</p>
{{ end }}
0 pkg/ui/chooser/embed/css/simple.min.css → ...i/chooser/embed/static/css/simple.min.css
View file
Open in desktop
File renamed without changes.
0 pkg/ui/chooser/embed/favicon.ico → pkg/ui/chooser/embed/static/favicon.ico
View file
Open in desktop
File renamed without changes.
0 pkg/ui/chooser/embed/media/logo-dark.svg → .../chooser/embed/static/media/logo-dark.svg
View file
Open in desktop
File renamed without changes
0 pkg/ui/chooser/embed/media/logo-light.svg → ...chooser/embed/static/media/logo-light.svg
View file
Open in desktop
File renamed without changes
6 changes: 1 addition & 5 deletions pkg/ui/chooser/middleware/authorization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,7 @@ func NewUserMiddleware(userPermissions *models.UserPermissions) fiber.Handler {
return ctx.Next()
}
if authToken == nil || !authToken.HasUserAccess(namespace.Code) {
return ctx.Status(
http.StatusNotFound,
).SendString(
"unable to find requested resource",
)
return ctx.Redirect("/errors/not-found", http.StatusMovedPermanently)
}

ctx.Locals(authTokenContextKey, authToken)
Expand Down
6 changes: 5 additions & 1 deletion pkg/ui/chooser/routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func (r Router) Init(router fiber.Router) error {

// app for template rendering
app := fiber.New(fiber.Config{
Views: html.NewFileSystem(http.FS(sub), ".html"),
Views: html.NewFileSystem(http.FS(sub), ".html"),
ViewsLayout: "layouts/main",
})
router.Mount("/", app)

Expand All @@ -62,5 +63,8 @@ func (r Router) Init(router fiber.Router) error {
app.Get("/chooser/namespaces", r.controller.ListNamespaces)
app.Get("/chooser/namespaces/current", r.controller.GetCurrentNamespace)

errors := app.Group("errors")
errors.Get("/not-found", r.controller.NotFoundError)

return nil
}