You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/03/10 09:50:11 UTC

[GitHub] [couchdb] garrensmith opened a new pull request #2649: couch_jobs resubmit updates job data

garrensmith opened a new pull request #2649: couch_jobs resubmit updates job data
URL: https://github.com/apache/couchdb/pull/2649
 
 
   ## Overview
   
   When a job is either pending or finished and the job is resubmitted
   with new data the job data is updated.
   
   ## Testing recommendations
   
   Tests should pass
   
   ## Related Issues or Pull Requests
   
   This is needed for #2585 
   
   ## Checklist
   
   - [x] Code is written and works correctly
   - [x] Changes are covered by tests
   - [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] garrensmith merged pull request #2649: couch_jobs resubmit updates job data

Posted by GitBox <gi...@apache.org>.
garrensmith merged pull request #2649: couch_jobs resubmit updates job data
URL: https://github.com/apache/couchdb/pull/2649
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] nickva commented on a change in pull request #2649: couch_jobs resubmit updates job data

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2649: couch_jobs resubmit updates job data
URL: https://github.com/apache/couchdb/pull/2649#discussion_r390363753
 
 

 ##########
 File path: src/couch_jobs/test/couch_jobs_tests.erl
 ##########
 @@ -426,6 +429,40 @@ resubmit_enqueues_job(#{t1 := T, j1 := J}) ->
     end).
 
 
+resubmit_pending_updates_job_data(#{t1 := T, j1 := J}) ->
+    ?_test(begin
+        Data1 = #{<<"test">> => 1},
+        Data2 = #{<<"test">> => 2},
+        ok = couch_jobs:add(?TX, T, J, Data1),
+        ?assertEqual(ok, couch_jobs:add(?TX, T, J, Data2, 6)),
+        ?assertMatch({ok, _, Data2}, couch_jobs:accept(T))
+    end).
+
+
+resubmit_finished_updates_job_data(#{t1 := T, j1 := J}) ->
+    ?_test(begin
+        Data1 = #{<<"test">> => 1},
+        Data2 = #{<<"test">> => 2},
+        ok = couch_jobs:add(?TX, T, J, Data1),
+        {ok, Job1, #{}} = couch_jobs:accept(T),
+        ?assertEqual(ok, couch_jobs:finish(?TX, Job1)),
+        ?assertEqual(ok, couch_jobs:add(?TX, T, J, Data2, 6)),
+        ?assertMatch({ok, _, Data2}, couch_jobs:accept(T))
+    end).
+
+
+resubmit_running_does_not_update_job_data(#{t1 := T, j1 := J}) ->
+    ?_test(begin
+        Data1 = #{<<"test">> => 1},
+        Data2 = #{<<"test">> => 2},
+        ok = couch_jobs:add(?TX, T, J, Data1),
+        {ok, Job1, #{}} = couch_jobs:accept(T),
+        ?assertEqual(ok, couch_jobs:add(?TX, T, J, Data2, 6)),
+        ?assertEqual(ok, couch_jobs:finish(?TX, Job1)),
+        ?assertMatch({ok, _, Data1}, couch_jobs:accept(T))
+    end).
+
+
 
 Review comment:
   Let's rename these to `add_...` since they go through `add/5` API only and also have `resubmit_` versions that go through `resubmit` API as well. So first is an `add/4,5` then we call `resubmit/4`. It may be redundant but it it is in case we change either API behavior in the future we'd want to see if the behavior changed for them individually.
   
   For the resubmit cases also let's check how resubmit with `undefined` data works in those states.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] nickva commented on a change in pull request #2649: couch_jobs resubmit updates job data

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2649: couch_jobs resubmit updates job data
URL: https://github.com/apache/couchdb/pull/2649#discussion_r390355349
 
 

 ##########
 File path: src/couch_jobs/src/couch_jobs_fdb.erl
 ##########
 @@ -205,8 +206,11 @@ finish(#{jtx := true} = JTx0, #{jlock := <<_/binary>>} = Job, Data) when
             {error, halt}
     end.
 
+resubmit(JTx0, Job, NewSTime) ->
 
 Review comment:
   Let's switch it such that the new `Data` parameter is at the end of the parameter list.
   
   So the resubmit version look like:
   
   ```
   resubmit(Tx, Job, SchedTime).
   resubmit(Tx, Job, SchedTime, Data)
   ```
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [couchdb] nickva commented on a change in pull request #2649: couch_jobs resubmit updates job data

Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #2649: couch_jobs resubmit updates job data
URL: https://github.com/apache/couchdb/pull/2649#discussion_r390487298
 
 

 ##########
 File path: src/couch_jobs/test/couch_jobs_tests.erl
 ##########
 @@ -426,6 +431,30 @@ resubmit_enqueues_job(#{t1 := T, j1 := J}) ->
     end).
 
 
+resubmit_finished_updates_job_data(#{t1 := T, j1 := J}) ->
 
 Review comment:
   Let's do one more case where we resubmit but with undefined data (resubmit/3) and then data should not be updated.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services