Skip to content

Conversation

@gruselhaus
Copy link
Contributor

@gruselhaus gruselhaus commented Nov 15, 2021

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=2023356
https://issues.redhat.com/browse/OCPBUGSM-37199

Analysis / Root cause:

In Safari 15.x on macOS, the Devfiles samples can't be loaded from the backend. As viewed in the network inspector of the browser I can say that the app makes two calls to the backend. The first answer was a redirect, the second the failure. Unfortunately, the payload gets lost between the first and second call.

Solution Description:

I changed the http method from put to get and passed the registry via an url param instead of a payload since we are fetching data from the backend only.

Screen shots / Gifs for design review:

Before:

=?UTF-8?Q?Bildschirmfoto_2021-11-15_um_15=2E08=2E12=2Epng?=

141813321-16e7627e-198a-440b-92ab-e5850b6d3cbf

After:

Bildschirmfoto 2021-11-15 um 19 37 20

Bildschirmfoto 2021-11-15 um 19 37 29

Unit test coverage report:

Test setup:
Version-Release number of selected component (if applicable):
4.9

How reproducible:
Always

Steps to Reproduce:

  1. Switch to dev console
  2. Create a new project
  3. Click on "+Add" or "View all samples"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

@openshift-ci openshift-ci bot added component/backend Related to backend component/dev-console Related to dev-console labels Nov 15, 2021
@gruselhaus
Copy link
Contributor Author

/test e2e-gcp-console

@christoph-jerolimov
Copy link
Member

/ok-to-test
/retitle Bug 2023356: Devfiles can't be loaded in Safari on macOS (403 - Forbidden)

@openshift-ci openshift-ci bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Nov 16, 2021
@openshift-ci openshift-ci bot changed the title Bug 2023356 - Devfiles can't be loaded in Safari on macOS (403 - Forbidden) Bug 2023356: Devfiles can't be loaded in Safari on macOS (403 - Forbidden) Nov 16, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 16, 2021

@gruselhaus: This pull request references Bugzilla bug 2023356, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 2023356: Devfiles can't be loaded in Safari on macOS (403 - Forbidden)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 16, 2021
@christoph-jerolimov
Copy link
Member

/bugzilla refresh

@christoph-jerolimov
Copy link
Member

/assign

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 16, 2021

@jerolimov: An error was encountered querying GitHub for users with public email (spathak@redhat.com) for bug 2023356 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message. non-200 OK status code: 403 Forbidden body: "{\n \"documentation_url\": \"https://docs.github.com/en/free-pro-team@latest/rest/overview/resources-in-the-rest-api#secondary-rate-limits\",\n \"message\": \"You have exceeded a secondary rate limit. Please wait a few minutes before you try again.\"\n}\n"

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@christoph-jerolimov
Copy link
Member

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 16, 2021

@jerolimov: An error was encountered querying GitHub for users with public email (spathak@redhat.com) for bug 2023356 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message. non-200 OK status code: 403 Forbidden body: "{\n \"documentation_url\": \"https://docs.github.com/en/free-pro-team@latest/rest/overview/resources-in-the-rest-api#secondary-rate-limits\",\n \"message\": \"You have exceeded a secondary rate limit. Please wait a few minutes before you try again.\"\n}\n"

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gruselhaus
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 16, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 16, 2021

@gruselhaus: This pull request references Bugzilla bug 2023356, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sanketpathak

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot requested a review from sanketpathak November 16, 2021 10:56
@gruselhaus
Copy link
Contributor Author

/retest

@gruselhaus
Copy link
Contributor Author

/test frontend

gitopsEndpoint = "/api/gitops/"
devfileEndpoint = "/api/devfile/"
devfileSamplesEndpoint = "/api/devfile/samples/"
devfileSamplesEndpoint = "/api/devfile/samples"

Choose a reason for hiding this comment

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

This is actually the fix, but when I discussed this issue with @gruselhaus, we agreed that its not required to make a PUT call for fetching the devfile data. So the rest of the PR changes the PUT Api call to GET. 👍

Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

Verified this PR with a cluster bot instance on Chrome, Firefox and Safari (all on macOS)

Works fine 👍

/approved
/lgtm
/retest

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2021
@christoph-jerolimov
Copy link
Member

/cc @jhadvig @florkbr
Jakub or Bryan, can one of you please take look at the go changes? Thanks :)

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@gruselhaus
Copy link
Contributor Author

@jerolimov the e2e-gcp-console still fails. Is there a way to bypass this test so Tide can merge this PR?

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@karthikjeeyar
Copy link
Contributor

/retest

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 23, 2021

@gruselhaus: An error was encountered querying GitHub for users with public email (spathak@redhat.com) for bug 2023356 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message. non-200 OK status code: 403 Forbidden body: "{\n \"documentation_url\": \"https://docs.github.com/en/free-pro-team@latest/rest/overview/resources-in-the-rest-api#secondary-rate-limits\",\n \"message\": \"You have exceeded a secondary rate limit. Please wait a few minutes before you try again.\"\n}\n"

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

Bug 2023356: Devfiles can't be loaded in Safari on macOS (403 - Forbidden)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gruselhaus
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 23, 2021

@gruselhaus: This pull request references Bugzilla bug 2023356, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sanketpathak

Details

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit e901f09 into openshift:master Nov 23, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 23, 2021

@gruselhaus: All pull requests linked via external trackers have merged:

Bugzilla bug 2023356 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2023356: Devfiles can't be loaded in Safari on macOS (403 - Forbidden)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gruselhaus gruselhaus deleted the fix-2023356 branch November 23, 2021 17:14
@gruselhaus
Copy link
Contributor Author

/cherry-pick release-4.9

@openshift-cherrypick-robot

@gruselhaus: new pull request created: #10546

Details

In response to this:

/cherry-pick release-4.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@spadgett spadgett added this to the v4.10 milestone Dec 8, 2021
gruselhaus added a commit to yangcao77/console that referenced this pull request Dec 10, 2021
yangcao77 pushed a commit to yangcao77/console that referenced this pull request Dec 11, 2021
meysam-jeffrey pushed a commit to snapp-incubator/console that referenced this pull request Dec 24, 2025
meysam-jeffrey pushed a commit to snapp-incubator/console that referenced this pull request Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. component/backend Related to backend component/dev-console Related to dev-console lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants