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

Fix: assets path for index.js query #412

Merged
merged 3 commits into from
Apr 21, 2022
Merged

Fix: assets path for index.js query #412

merged 3 commits into from
Apr 21, 2022

Conversation

anrusina
Copy link
Contributor

@anrusina anrusina commented Apr 20, 2022

We had an issue raised by one of our users who had different domains for console and admin api.

In his case all Admin API request performed at ui_domain/console where navigated to ui_domain/api/v1/request instead of admin_api/api/v1/request.
However if request performed from any other page , for example domain/console/projects/flytectldemo/workflows it will work as expect.

This change adds back /assets folder to hold main.js and vendor.js and puts assets at the same /assets folder

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

Alex Bain (Level 5 US)/Alex Bain and others added 2 commits April 18, 2022 17:55
Signed-off-by: Alex Bain (Level 5 US)/Alex Bain <[email protected]>
Signed-off-by: Nastya Rusina <[email protected]>
@anrusina anrusina requested review from a team, ursucarina, jsonporter and olga-union and removed request for a team April 20, 2022 17:53
@codecov
Copy link

codecov bot commented Apr 20, 2022

Codecov Report

Merging #412 (8bf831e) into master (0c0cd4b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #412   +/-   ##
=======================================
  Coverage   67.08%   67.08%           
=======================================
  Files         407      407           
  Lines        9147     9147           
  Branches     1614     1614           
=======================================
  Hits         6136     6136           
  Misses       3011     3011           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2995f3c...8bf831e. Read the comment docs.

@anrusina
Copy link
Contributor Author

@convexquad If you can check it on your side it would be amazing

@convexquad
Copy link

I built and tested in my EKS cluster and everything is working for me

@anrusina anrusina changed the title Narusina/assets Fix: assets path for index.js query Apr 21, 2022
Copy link
Contributor

@olga-union olga-union left a comment

Choose a reason for hiding this comment

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

looks good

@anrusina anrusina merged commit 940116f into master Apr 21, 2022
@anrusina anrusina deleted the narusina/assets branch April 21, 2022 01:28
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 0.54.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants