-
Notifications
You must be signed in to change notification settings - Fork 6
Fix/scr 009 #232
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
Merged
Merged
Fix/scr 009 #232
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ns for SSR and graphql subscriptions. Direct subscription are automatically formed from hostname
… WEBSOCKET_URL from .env.development
…ns for SSR and graphql subscriptions. Direct subscription are automatically formed from hostname
… WEBSOCKET_URL from .env.development
thomasimda
approved these changes
Dec 15, 2023
imda-benedictlee
approved these changes
Dec 26, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve & Reviewed Code
Smoke Test for Docker Passed. Report Attached: |
Pipeline Test for Docker Passed. Screenshot Attached. |
1 task
iamksuresh
pushed a commit
that referenced
this pull request
Nov 13, 2024
Fix/scr 009 The requested reviews are Approved. Proceeded to merge PR to Main.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Update the portal graphql client so that all browser requests uses the portal rewrites for graphql client requests and subscriptions. Graphql URL are automatically formatted using the window.location hostname and port, so that WS connections connect to the right hostname instead of default localhost.
This fix should also resolve issue: #169
The follow variables are remove from portal .env.development:
NEXT_PUBLIC_SERVER_URL, NEXT_PUBLIC_WEBSOCKET_URL, SERVER_URL, WEBSOCKET_URL
Only NEXT_PUBLIC_WEBSOCKET_URL is still in use but it should be set only if the websocket URL need to be hardcoded.
Motivation and Context
[Explain the motivation or the context behind this pull request. Why is it necessary?]
Type of Change
fix:
The portal graphql URL should now be set correctly without having to set the SEVER_URL and WEBSOCKET_URL env.
How to Test
Deploy and run the ai-verify-apigw and ai-verify-portal on one server. Access the portal from another PC or laptop and observe the http requests to make sure there's no connection errors.
Checklist
Please check all the boxes that apply to this pull request using "x":
Screenshots (if applicable)
[If the changes involve visual modifications, include screenshots or GIFs that demonstrate the changes.]
Additional Notes
[Add any additional information or context that might be relevant to reviewers.]
Developer Certificate of Origin