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.

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

frontend: Fix 500 error after detail page refresh (#1967) #2001

Conversation

elenzio9
Copy link
Contributor

@elenzio9 elenzio9 commented Nov 7, 2022

Fix 500 error when refreshing KWA's detail page by also adding the namespace variable as a query parameter to the route.

Related issue: #1967

Signed-off-by: Elena Zioga elena@arrikto.com

@elenzio9 elenzio9 force-pushed the feature-elena-kwa-detail-page-error-fix branch from 9b49362 to 9e2ed03 Compare November 7, 2022 13:40
@coveralls
Copy link

coveralls commented Nov 7, 2022

Coverage Status

Coverage increased (+0.3%) to 73.737% when pulling 9e2ed03 on arrikto:feature-elena-kwa-detail-page-error-fix into c25518a on kubeflow:master.

@johnugeorge
Copy link
Member

Thanks @elenzio9

@shaowei-su Can you check this out in your env if it works ?

@kimwnasptd
Copy link
Member

@elenzio9 thanks for the PR!

With a quick glance I see that you only changed the files of the page that handles an Experiment. Should we also change the main page with the table that redirects users to the details page of an Experiment? To ensure the correct link is used there as well

@elenzio9
Copy link
Contributor Author

elenzio9 commented Nov 7, 2022

@elenzio9 thanks for the PR!

With a quick glance I see that you only changed the files of the page that handles an Experiment. Should we also change the main page with the table that redirects users to the details page of an Experiment? To ensure the correct link is used there as well

I've also updated this page as you can see here: https://github.com/kubeflow/katib/pull/2001/files#diff-4e45400d0d644ac998f731d708df6edd6667d51b19c7be069e2522dfcef3f137R89

@shaowei-su
Copy link
Contributor

Thanks @elenzio9 @johnugeorge , just tested this branch offline and the 500 error is fixed. Huge thanks for your timely fix 👍

@shaowei-su
Copy link
Contributor

shaowei-su commented Nov 7, 2022

Actually, I found an issue with loading the visualized graph if the experiment is in "running" state (6 out of 10 trials are already succeeded but failed to load) :
Screen Shot 2022-11-07 at 15 37 50

@elenzio9
Copy link
Contributor Author

elenzio9 commented Nov 8, 2022

Actually, I found an issue with loading the visualized graph if the experiment is in "running" state (6 out of 10 trials are already succeeded but failed to load)

Since the above issue is related to the experiment graph, it would be helpful to create another issue and describe everything there. Thank you!

@johnugeorge
Copy link
Member

@

Actually, I found an issue with loading the visualized graph if the experiment is in "running" state (6 out of 10 trials are already succeeded but failed to load) :
Screen Shot 2022-11-07 at 15 37 50

@shaowei-su Can you create a separate issue to track this? This PR is good to go

/assign @kimwnasptd

@kimwnasptd
Copy link
Member

@elenzio9 can you do a rebase here as well to ensure the unittests are passing as well?

Fix 500 error when refreshing KWA's detail page by also adding the
namespace variable as a query param to the route.

Signed-off-by: Elena Zioga <elena@arrikto.com>
@elenzio9 elenzio9 force-pushed the feature-elena-kwa-detail-page-error-fix branch from 9e2ed03 to 8bd5d42 Compare November 11, 2022 15:58
@kimwnasptd
Copy link
Member

/lgtm
/approve

@google-oss-prow google-oss-prow bot added the lgtm label Nov 14, 2022
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elenzio9, kimwnasptd

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 9e0e173 into kubeflow:master Nov 14, 2022
@kimwnasptd kimwnasptd deleted the feature-elena-kwa-detail-page-error-fix branch November 14, 2022 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants