-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: Console Pod Logs broken if not in a plural application #348
Conversation
Visit the preview URL for this PR (updated for commit cb75b75): https://pluralsh-console--pr348-klink-pod-logs-fix-rzyrdrfw.web.app (expires Thu, 04 May 2023 20:38:56 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: dd1ffa0705acc6ef7d6db370e6bd6fc390e945ce |
name?: string | ||
namespace?: string | ||
}) { | ||
const isApp = useNamespaceIsApp(namespace) |
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.
could we just pass this as an argument and ignore the potentially risky app name fetch?
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.
Is there some field on a Pod that I could use to determine if it's an app or not? Otherwise I'd just be pushing the app name fetch further up the render tree.
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.
Not easily, I thought you might be able to determine it from context of where the component is called, eg if it's called from an /apps/
prefixed url use something like that link, otherwise non-app link?
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.
Nah, I need to know whether it's an app or not on the Pod page, where it's not obvious.
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.
Either that, or we always show the logs on the Pod page, even if there is an associated App page where you could also view the logs.
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.
Let's just try this for now, it should be accurate enough
Summary
Created a new 'Logs' tab for Pods pages that have no associated apps. Pods without an associated apps will have the 'View logs' button direct to this tab while pods with apps will still be linked to the App page's logs tab.
Labels
Test Plan
Checklist