-
Notifications
You must be signed in to change notification settings - Fork 167
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
refactor: add -pricePerGateway and deprecate -pricePerBroadcaster #3056
Conversation
5f80ee7
to
c6b9662
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3056 +/- ##
===================================================
+ Coverage 57.39517% 57.40400% +0.00883%
===================================================
Files 92 92
Lines 15740 15755 +15
===================================================
+ Hits 9034 9044 +10
- Misses 6104 6109 +5
Partials 602 602
Continue to review full report in Codecov by Sentry.
|
Looks good, could you check why the test suite is failing before merging though please |
This commit adds the `pricePerGateway` flag and deprecates the `pricePerBroadcaster` flag per core team decision (details: https://discord.com/channels/423160867534929930/1051963444598943784/1210356864643109004).
This commit removes the `PricePerBroadcaster` deprecation comment since this is already clear from the glog warning below.
This commit ensures that the deprecation condition for the `pricePerBroadcaster` flag properly handles the default empty string value.
This commit ensures that the `pricePerGateway` is correctly used instead of the `pricePerBroadcaster` when it is set.
…of 'gateways' This commit updates the configuration to replace the `broadcasters` property specified under the `pricePerGateway` flag with `gateways`. Additionally, it ensures that a warning is issued when the deprecated property is still used.
6c1df80
to
74fa79e
Compare
This commit ensures that the TestParseGetBroadcasterPrices function uses the new getGatewayPrices function.
This commit updates the `TestParseGetBroadcasterPrices` function to `TestParseGetGatewayPrices` to align with the new node naming convention.
I reviewed the test errors on my local machine and compared them with those in the GitHub Action. On my local system, the following test fails: I checked out the latest stable release (v0.7.5), and the same error occurs locally: Error Trace: /home/ricks/development/work/livepeer/ai_spe/go-livepeer/server/segment_rpc_test.go:1723
Error: Should have called with given arguments
Test: TestSubmitSegment_HttpPostError
Messages: Expected "Credit" to have been called with:
[5/1]
but actual calls were:
[0/1 0/1] This happens because the SubmitSegment function throws the following error: E0519 22:07:08.094493 2971643 segment_rpc.go:517] orchSessionID=bar orchestrator=https://127.0.0.1 Error submitting segment code=404 orch=https://127.0.0.1 err="<html>\r\n<head><title>404 Not Found</title></head>\r\n<body>\r\n<center><h1>404 Not Found</h1></center>\r\n<hr><center>nginx/1.18.0 (Ubuntu)</center>\r\n</body>\r\n</html>\r\n" Strangely, the end-to-end test that fails in the action is successful on my local machine. 🤔 I believe we are good to merge since these issues are not caused by my code. Can you merge this pull request if you agree? |
The local error was easy to fix. I discovered that an Nginx server was running on my machine, automatically starting at boot. Shutting it down resolved the issue, allowing the tests for this pull request to succeed on my local machine 👍🏻. |
Merged as all tests were successful 👍🏻. |
What does this pull request do? Explain your changes. (required)
This pull request adds the
pricePerGateway
flag and deprecates thepricePerBroadcaster
flag per core team decision (details: https://discord.com/channels/423160867534929930/1051963444598943784/1210356864643109004). It adds the new-pricePerGateway
flag while maintaining support for-pricePerBroadcaster
with a deprecation warning on startup. This change does not address refactoring associated with variable names and types, which will be addressed in a later change.Specific updates (required)
-pricePerGateway
flaggetBroadcasters
function to parse thegateways
property instead of thebroadcasters
property.W0516 11:37:39.563931 97531 starter.go:785] -PricePerBroadcaster flag is deprecated and will be removed in a future release. Please use -PricePerGateway instead
How did you test each of these updates (required)
-pricePerGateway
flag-pricePerBroadcaster
flag, noted deprecation warningbroadcasters
property in the config specified in the-pricePerGateway
flag, noted deprecation warning.Does this pull request close any open issues?
AI-LIV-358
Checklist:
make
runs successfully./test.sh
passTestSubmitSegment_HttpPostError
server test does fails on my system but I don't think it is related to my changesTest Logs