Schedule command delivering, update version to `0.3` #123

AlexanderLitus · 2016-04-07T12:34:29Z

Also updated version to 0.3.

codecov-io · 2016-04-07T12:37:14Z

Current coverage is `83.46%`

Merging #123 into master will increase coverage by +1.37% as of 18b425e

@@            master    #123   diff @@
======================================
  Files           95      97     +2
  Stmts         2692    2794   +102
  Branches       318     331    +13
  Methods                          
======================================
+ Hit           2210    2332   +122
- Partial         79      81     +2
+ Missed         403     381    -22

Review entire Coverage Diff as of 18b425e

Powered by Codecov. Updated on successful CI builds.

AlexanderLitus · 2016-04-07T13:21:24Z

@alexander-yevsyukov, PTAL.

alexander-yevsyukov · 2016-04-07T15:01:05Z

client/src/main/proto/spine/base/command.proto

    Namespace namespace = 5;
+
+    oneof time_opts {
+        // The delay to wait before sending the command.


The delay after which moment?

After the current time.

Conflicts: client/src/main/java/org/spine3/base/Commands.java client/src/test/java/org/spine3/base/CommandsShould.java examples/build.gradle server/src/main/java/org/spine3/server/command/CommandBus.java

add a builder to CommandBus.

AlexanderLitus · 2016-04-08T11:54:19Z

@alexander-yevsyukov, PTAL.

alexander-yevsyukov · 2016-04-08T20:01:46Z

client/src/main/proto/spine/base/command.proto


-        // The time when the command should be sent.
-        google.protobuf.Timestamp sending_time = 7;
+        // The time when the command should be delivered to the target.


The delivery time depends on the path a command goes after it is posted to CommandBus. There could be one or more dispatchers in between, for example. The load time for an aggregate may be unpredictable, etc.

So, in general, we won't be able to guarantee the delivery by the time specified. It would be too complex. What we can promise is that we'd post the command to the CommandBus shortly after the time specified.

How about just after?

OK, so I'm removing delivery_time for now.

…r fields.

AlexanderLitus · 2016-04-18T10:01:21Z

@alexander-yevsyukov, PTAL.

alexander-yevsyukov · 2016-04-18T13:52:08Z

client/src/main/proto/spine/base/command.proto

-    // This flag is needed to specify that a scheduled command arrived in time, and to retain the scheduling options.
-    bool in_time = 3;
+    // This flag is needed to specify that the delay is passed, and to retain the info about the delay.
+    bool ignore_delay = 2;


This name is still confusing. Why cannot we have a set of already scheduled command IDs? If a command posted is already scheduled, it would mean that we need to execute it.

AlexanderLitus · 2016-04-18T15:24:37Z

PTAL.

alexander-yevsyukov · 2016-04-18T17:21:39Z

server/src/test/java/org/spine3/server/command/ExecutorCommandSchedulerShould.java


-        assertCommandSent(cmd, DELAY);
+        assertNull(postedCommand);
+        sleep(DELAY_MS + CHECK_OFFSET_MS);


Please test it using spy() or mock(). Sleeping threads does not scale well for testing.

AlexanderLitus · 2016-04-19T10:16:47Z

PTAL.

Conflicts: server/src/main/java/org/spine3/server/command/CommandBus.java server/src/test/java/org/spine3/server/BoundedContextShould.java server/src/test/java/org/spine3/server/BoundedContextTestStubs.java server/src/test/java/org/spine3/server/command/CommandBusShould.java

Conflicts: server/src/main/java/org/spine3/server/command/CommandBus.java

alexander-yevsyukov · 2016-04-19T15:21:45Z

client/src/main/java/org/spine3/protobuf/Durations.java

 /**
 * Utility class for working with durations in addition to those available from {@link TimeUtil}.
- * 
+ * <p/>


Please remove the excessive paragraph tag.

Can we make IDEA stop adding these nasty tags?

alexander-yevsyukov · 2016-04-19T15:24:44Z

server/build.gradle

-                grpc {
-//                    option 'nano=true'
-                }
+                grpc {}


Do we need the empty grpc block?

We do. Adding even empty closure in this section adds protoc option --%CLOSURE_NAME%_out. In our case, this empty block is a signal to generate grpc services

alexander-yevsyukov · 2016-04-19T15:30:04Z

values/build.gradle

-                grpc {
-//                    option 'nano=true'
-                }
+                grpc {}


Do we need the empty grpc section here?

No, we don't

Conflicts: client/src/main/java/org/spine3/base/Responses.java server/src/main/java/org/spine3/server/command/CommandBus.java server/src/main/java/org/spine3/server/reflect/MethodMap.java server/src/test/java/org/spine3/server/BoundedContextShould.java server/src/test/java/org/spine3/server/BoundedContextTestStubs.java server/src/test/java/org/spine3/server/command/CommandBusShould.java values/build.gradle

AlexanderLitus · 2016-04-19T15:51:05Z

PTAL.

alexander-yevsyukov · 2016-04-19T15:59:58Z

Please use times(1) instead of atMost(1) in tests. The change seems to be lost in recent merges.

alexander-yevsyukov · 2016-04-19T16:00:24Z

Other than that, LGTM

Alexander Litus added 3 commits April 6, 2016 17:08

Remove empty lines.

d2b5290

Use checkNotDefault() method.

f4dc5ff

Add command scheduler to command bus.

18b04be

AlexanderLitus assigned alexander-yevsyukov Apr 7, 2016

Alexander Litus added 2 commits April 7, 2016 16:03

Fix command bus tests.

e0e8ecc

Add more tests.

c4a6e1b

Update protobuf plugin dependency.

637183f

alexander-yevsyukov reviewed Apr 7, 2016
View reviewed changes

Alexander Litus added 2 commits April 8, 2016 10:49

Merge branch 'master' into schedule-commands

76ce390

Conflicts: client/src/main/java/org/spine3/base/Commands.java client/src/test/java/org/spine3/base/CommandsShould.java examples/build.gradle server/src/main/java/org/spine3/server/command/CommandBus.java

Make CommandScheduler an abstract class; add docs, tests;

82894c4

add a builder to CommandBus.

alexander-yevsyukov reviewed Apr 8, 2016
View reviewed changes

Alexander Litus added 4 commits April 18, 2016 11:51

Remove delivery_time field in command Schedule options, rename othe…

3598407

…r fields.

Improve an error msg.

970ff8f

Rename a constant.

292f893

Avoid circular dependency between CommandBus and CommandScheduler.

96ade6e

Alexander Litus added 3 commits April 18, 2016 13:35

Use mockito spy instead of mock to improve code coverage.

bfdcb38

Merge branch 'master' into schedule-commands

8689455

Add test API to aggregate.

2b42bcf

alexander-yevsyukov reviewed Apr 18, 2016
View reviewed changes

Track commands which are scheduled already in scheduler.

754a161

alexander-yevsyukov reviewed Apr 18, 2016
View reviewed changes

Alexander Litus added 2 commits April 19, 2016 13:14

Improve command schedule tests.

b1f115d

Rename aggregate methods for tests.

951b8b5

Alexander Litus added 5 commits April 19, 2016 13:33

Fix tests.

4aeca10

Merge branch 'master' into schedule-commands

9b9674f

Conflicts: server/src/main/java/org/spine3/server/command/CommandBus.java

Merge branch 'master' into this one.

a2567ac

0.3 version.

5be6343

alexander-yevsyukov reviewed Apr 19, 2016
View reviewed changes

Update protobuf-plugin dep-cy.

203390e

alexander-yevsyukov reviewed Apr 19, 2016
View reviewed changes

Remove redundant tag in docs.

5384729

alexander-yevsyukov reviewed Apr 19, 2016
View reviewed changes

Alexander Litus added 3 commits April 19, 2016 18:33

Minor changes.

9193013

Remove redundant code.

e225e12

Alexander Litus added 2 commits April 20, 2016 11:49

Add a method to Responses.

cdb9a1e

Fix and add command bus tests.

e0e8215

AlexanderLitus merged commit 1e19970 into master Apr 20, 2016

AlexanderLitus deleted the schedule-commands branch April 20, 2016 09:03

AlexanderLitus changed the title ~~Schedule command delivering~~ Schedule command delivering, update version to 0.3 Jun 7, 2016

Schedule command delivering, update version to 0.3 #123

Schedule command delivering, update version to 0.3 #123

Uh oh!

Conversation

AlexanderLitus commented Apr 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Apr 7, 2016

Current coverage is 83.46%

Uh oh!

AlexanderLitus commented Apr 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLitus commented Apr 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLitus commented Apr 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLitus commented Apr 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLitus commented Apr 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLitus commented Apr 19, 2016

Uh oh!

alexander-yevsyukov commented Apr 19, 2016

Uh oh!

alexander-yevsyukov commented Apr 19, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Schedule command delivering, update version to `0.3` #123

Schedule command delivering, update version to `0.3` #123

AlexanderLitus commented Apr 7, 2016 •

edited

Loading

Current coverage is `83.46%`