-
Notifications
You must be signed in to change notification settings - Fork 173
fix: update console output to show both host and port #2953
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: update console output to show both host and port #2953
Conversation
…ostPort Signed-off-by: RayyanSeliya <[email protected]>
|
Welcome @RayyanSeliya! It looks like this is your first PR to knative/func 🎉 |
|
Hi @RayyanSeliya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
/ok-to-test |
Signed-off-by: RayyanSeliya <[email protected]>
|
hey @gauron99 i was getting fail on e2e test specifying a mismatch on the text display like before we have like Running on host port... and now we have Function running on ... so the test was failing can retrigger the test ! thx ! Sorry still failing I think more logic is required separate extraction of host and port coz it is getting stuck on url thing as I can see on the test any suggestions? |
|
@RayyanSeliya you also need to update test, https://github.com/knative/func/blob/3641bfe5d03af71fcbfaa3065b90198da13646c3/test/e2e/scenario_extended_flow_test.go#L70-L69. You need to update the regex there. |
Thx @matejvasek for helping me out i will update the regex in that test file you mentioned |
Signed-off-by: RayyanSeliya <[email protected]>
|
/ok-to-test |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2953 +/- ##
==========================================
+ Coverage 57.55% 60.03% +2.47%
==========================================
Files 132 132
Lines 16836 16836
==========================================
+ Hits 9690 10107 +417
+ Misses 6237 5771 -466
- Partials 909 958 +49
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matejvasek, RayyanSeliya The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
fixes #2900
net.JoinHostPortfor better host:port formattingWhy
The previous implementation only showed the port number in the console output, which could be confusing when running in different environments. The new implementation:
net.JoinHostPortfor consistent host:port formattingTesting
The changes were tested by:
see the output of testing
rayyan@rayyan-seliya:/mnt/c/Users/RAYYAN/Desktop/func/myfunction$ ../func run --address=127.0.0.1:8081 {"level":"debug","time":1753544409,"message":"func runtime creating function instance"} {"level":"debug","address":"127.0.0.1:8081","time":1753544409,"message":"function starting"} {"level":"debug","time":1753544409,"message":"function does not implement Start. Skipping"} {"level":"debug","time":1753544409,"message":"waiting for stop signals or errors"} Function running on 127.0.0.1:8081Release Note