Skip to content

fix: set websocket when passed true and add websocket e2e test #2497

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
Merged
Revolyssup merged 6 commits into from
Jul 30, 2025
Merged

fix: set websocket when passed true and add websocket e2e test #2497

Revolyssup merged 6 commits into from
Jul 30, 2025

Conversation

@Revolyssup
Copy link
Contributor

@Revolyssup Revolyssup commented Jul 29, 2025 •
edited
Loading

Type of change:

Fixes #2499

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches
  • Documentation
  • Refactor
  • Chore
  • CI/CD or Tests

What this PR does / why we need it:

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@Revolyssup Revolyssup requested review from AlinsRan and ronething July 29, 2025 14:39
@ronething
Copy link
Contributor

ronething commented Jul 30, 2025

@Revolyssup Could we deal with this issue by the way? #2499 cc @nic-6443

@Revolyssup
Copy link
Contributor Author

Revolyssup commented Jul 30, 2025

@Revolyssup Could we deal with this issue by the way? #2499 cc @nic-6443

ohh i didn't see this logic. we should defintely change it from always being true. I will fix this here and add corresponding e2e case

@Revolyssup Revolyssup mentioned this pull request Jul 30, 2025
Closed
@Revolyssup
Copy link
Contributor Author

Revolyssup commented Jul 30, 2025

@ronething I have made the change and added test for it.

ronething
ronething approved these changes Jul 30, 2025
View reviewed changes
Copy link
Contributor

@ronething ronething left a comment

Choose a reason for hiding this comment

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

lgtm

@AlinsRan AlinsRan added the bugfix pull requests that fix a bug label Jul 30, 2025
@AlinsRan
Copy link
Contributor

AlinsRan commented Jul 30, 2025

This should be a fix, you need to update the PR title.

@Revolyssup Revolyssup changed the title chore: add websocket e2e test fix: set websocket when passed true and add websocket e2e test Jul 30, 2025
@Revolyssup Revolyssup merged commit 49ef9d4 into apache:master Jul 30, 2025
23 checks passed
@Revolyssup Revolyssup deleted the revolyssup/ingress-test-2 branch July 30, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ronething ronething ronething approved these changes

@AlinsRan AlinsRan AlinsRan approved these changes

Assignees

No one assigned

Labels

bugfix pull requests that fix a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: always set websocket as true when translate ApisixRoute

3 participants

@Revolyssup @ronething @AlinsRan