-
Notifications
You must be signed in to change notification settings - Fork 276
feat(demo): Update demo to work on OpenShift #2583
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2583 +/- ##
==========================================
+ Coverage 63.96% 64.10% +0.13%
==========================================
Files 151 151
Lines 6755 6755
==========================================
+ Hits 4321 4330 +9
+ Misses 2419 2410 -9
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
demo/cmd/common/books.go
Outdated
@@ -19,7 +19,7 @@ const ( | |||
RestockWarehouseURL = "restock-books" | |||
|
|||
// bookstorePort is the bookstore service's port | |||
bookstorePort = 80 | |||
bookstorePort = 8080 |
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.
I know this is a work in progress, but we shouldn't be changing to default demo ports to 8080 just for this. If for some reason this port is unusable for Openshift, we should simply override the port similar to other configurable options.
In the past we did change the demo ports to 8080 and back to 80, more details can be found here #1099 (comment)
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.
Updated in favor of using a different port
6a4119b
to
d2ca8ea
Compare
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.
I'll miss my goode olde port 80
To be addressed in #2755 :) |
Signed-off-by: Kalya Subramanian <kasubra@microsoft.com>
Signed-off-by: Kalya Subramanian kasubra@microsoft.com
Description:
This PR updates the ports in the automatic and manual demos to use a "random" port that will not be reserved by OpenShift.
In the future, we can make the port configurable.
Fixes #2744
Affected area:
Please answer the following questions with yes/no.
No