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

Jump to bottom

🐛? [Frontend bugfix?] Directly switch to service's iframe when its status is running #5948

Merged
merged 8 commits into from
Jun 17, 2024
Merged

🐛? [Frontend bugfix?] Directly switch to service's iframe when its status is running #5948

merged 8 commits into from
Jun 17, 2024

Conversation

odeimaiz
Copy link
Member

What do these changes do?

Related issue/s

How to test

Dev-ops checklist

@odeimaiz odeimaiz self-assigned this Jun 14, 2024
@odeimaiz odeimaiz requested review from jsaq007 and ignapas as code owners June 14, 2024 13:43
@odeimaiz odeimaiz changed the title 🐛? [frontend] Directly switch service's iframe when its status is running 🐛? [frontend] Directly switch service's iframe when its status is running Jun 14, 2024
@odeimaiz odeimaiz enabled auto-merge (squash) June 14, 2024 13:43
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jun 14, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

GitHK
GitHK approved these changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@GitHK GitHK left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Let's see what side effects this brings

@odeimaiz odeimaiz added the a:frontend issue affecting the front-end (area group) label Jun 14, 2024
elisabettai
elisabettai approved these changes Jun 14, 2024
View reviewed changes
Copy link
Collaborator

@elisabettai elisabettai left a comment

Choose a reason for hiding this comment

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

Thanks!

Shouldn't you add the "alarm" icon, since you think it's a risky one?

@odeimaiz odeimaiz changed the title 🐛? [frontend] Directly switch service's iframe when its status is running 🐛? [Frontend bugfix?] Directly switch service's iframe when its status is running Jun 14, 2024
@odeimaiz odeimaiz changed the title 🐛? [Frontend bugfix?] Directly switch service's iframe when its status is running 🐛? [Frontend bugfix?] Directly switch to service's iframe when its status is running Jun 14, 2024
pcrespov
pcrespov reviewed Jun 17, 2024
View reviewed changes
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

👍

sanderegg
sanderegg reviewed Jun 17, 2024
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

not sure I get the idea here. What will happen if the frontend is indeed not ready? will it retry?

@odeimaiz
Copy link
Member Author

not sure I get the idea here. What will happen if the frontend is indeed not ready? will it retry?

Not sure when that can happen, and not sure what will happen...

@odeimaiz odeimaiz disabled auto-merge June 17, 2024 07:57
@odeimaiz odeimaiz enabled auto-merge (squash) June 17, 2024 08:00
@sanderegg
Copy link
Member

not sure I get the idea here. What will happen if the frontend is indeed not ready? will it retry?

Not sure when that can happen, and not sure what will happen...

@odeimaiz
When you get the running state, that means the docker container says it is running, based on some healthcheck (if it is set which is not guaranteed and it might not be the same endpoint you are hitting BTW)
After that there is an unknown amount of time until Traefik routes the service so that the frontend can access it. so that is why we had that check.

@odeimaiz odeimaiz disabled auto-merge June 17, 2024 10:15
@odeimaiz odeimaiz enabled auto-merge (squash) June 17, 2024 12:33
@odeimaiz odeimaiz added this to the South Island Iced Tea milestone Jun 17, 2024
@odeimaiz odeimaiz merged commit f17ed4b into ITISFoundation:master Jun 17, 2024
54 checks passed
@YuryHrytsuk YuryHrytsuk mentioned this pull request Jun 20, 2024
Closed
14 tasks
@GitHK GitHK mentioned this pull request Jun 27, 2024
Closed
17 tasks
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jul 5, 2024
Closed
26 tasks
@odeimaiz odeimaiz deleted the maybefix/swith-to-iframes-content branch August 20, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanderegg sanderegg sanderegg left review comments

@pcrespov pcrespov pcrespov left review comments

@matusdrobuliak66 matusdrobuliak66 matusdrobuliak66 approved these changes

@elisabettai elisabettai elisabettai approved these changes

@GitHK GitHK GitHK approved these changes

@ignapas ignapas ignapas approved these changes

@jsaq007 jsaq007 Awaiting requested review from jsaq007 jsaq007 is a code owner

Assignees

@odeimaiz odeimaiz

Labels
a:frontend issue affecting the front-end (area group)
Projects
None yet
Milestone
South Island Iced Tea
Development

Successfully merging this pull request may close these issues.
7 participants
@odeimaiz @sanderegg @ignapas @GitHK @elisabettai @pcrespov @matusdrobuliak66