-
Notifications
You must be signed in to change notification settings - Fork 443
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
client: don't change user CPID if detach from oldest project #5179
Conversation
- Maintain a map from email hash to the oldest user CPID and its time. This doesn't change if that project is detached. - Write/parse write this in client_state.xml - Update it when handling a scheduler reply - Use it when sending a scheduler request - If, on startup, the map is empty, populate it from PROJECT data. This should happen only once, when update to this version
@KeithMyers, @makeasnek, any chance you could test this? |
I have a Windows 10 machine I can test it on if you get me an exe or installer :). |
@makeasnek, we don't have automatically built installer, but we have binaries. Here's a built client for this PR: https://github.com/BOINC/boinc/suites/12005652776/artifacts/630473582 |
Excellent thank you I will test this tomorrow |
I can build it. But how would you set up conditions to test if the fix works? I was thinking that you would have to already have split CPIDs in your client state file. Then run the new client and then see if the split CPIDs were converged to the oldest CPID Does that sound like the correct test scenario? |
In each of these tests, I am copying external CPID directly from client_state.xml file and reloading the file into notepad between checks Test 1: Test 2
So it looks like the intended functionality is working as removing the oldest project does not change CPID. A few things to note here:
|
@davidpanderson, could you please take a look at the test results above? |
I couldn't quite follow the part after "a few things to note".
The bottom line: CPIDs converge after 2 RPCs, which should happen pretty quickly. |
Can we not just issue an "update project" command/second RPC request automatically right after a project is added? That would solve the problem. Here's the scenario I'm envisioning:
|
For one thing, the code change you are requesting is in a completely different area of the client code. Do you want to wait some more for a "nice to have" change, or just get the main benefit of the originally asked for improvement merged? I don't think that a simple manual project update is too much to ask from the new user which achieves the same thing you ask. Or just wait for the second rpc that is going to happen regardless in a matter of minutes or at most a couple of hours. |
Actually it doesn't require a 2nd scheduler RPC |
…) RPC. That way (if we're already attached to a project with that email addr) the client can sent the appropriate user CPID in the initial scheduler RPC and the web site of the project will show it immediately.
Awesome thank you David! |
Thanks for the quick fix David. |
Seems to work. The test case is:
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #5179 +/- ##
=========================================
Coverage 10.86% 10.86%
Complexity 1064 1064
=========================================
Files 279 279
Lines 35993 35993
Branches 8258 8258
=========================================
Hits 3911 3911
Misses 31690 31690
Partials 392 392
|
@makeasnek, did you test the latest build? If not, please use these artifacts: |
Tested this situation with the latest build:
|
Maintain a map from email hash to the oldest user CPID and its time.
This doesn't change if that project is detached.
Write/parse write this in client_state.xml
Update it when handling a scheduler reply
Use it when sending a scheduler request
If, on startup, the map is empty, populate it from PROJECT data.
This should happen only once, when update to this version
Fixes #5177