Fix #2514 - Use optimistic concurrency to prevent multiple UpdateIsLatest calls on same package #3548

chenriksson · 2017-02-10T00:30:53Z

Approach is same as discussed, but implementation is more complex due to our need to restrict the optimistic concurrency check to CUD operations only on specific columns - which EF doesn't support.

joelverhagen · 2017-02-10T02:05:18Z

src/NuGetGallery/Services/PackageService.cs

@@ -253,7 +266,8 @@ where VersionRange.Parse(d.VersionSpec).Satisfies(packageVersion)

            await PublishPackageAsync(package, commitChanges);
        }
-
+
+        // caller is responsible for calling UpdateIsLatestAsync in separate transaction


This should be xmldoc so callers actually see it. Put it on the interface.

joelverhagen · 2017-02-10T02:07:16Z

src/NuGetGallery/Services/PackageService.cs

+            var altEntitiesContext = CreateNewEntitiesContext();
+
+            // suspend retry execution strategy which does not support user initiated transactions
+            EntitiesConfiguration.SuspendExecutionStrategy = true;


What is altEntitiesContext.GetDatabase(); throws? Won't this static stay true? In these cases, I like making an IDisposable which toggles the static on and off.

Another concern -- won't this effect other threads?

Shouldn't affect other threads. SuspendExecutionStrategy uses CallContext which is similar to thread local storage. Note that this is also used in PackageDeleteService, which also uses custom transactions.

I wasn't thinking about GetDatabase throwing, but perhaps it could if the connection was somehow closed. I'll just expand the scope of the try/finally, which I think will be less work than introducing a disposable.

joelverhagen · 2017-02-10T02:10:16Z

src/NuGetGallery/Services/PackageService.cs

+
+                            var packageRegistration = sets.First().Package.PackageRegistration;
+
+                            if (retryCount++ < UpdateIsLatestMaxRetries)


Please increment on a separate line. This is less readable.

joelverhagen · 2017-02-10T02:10:48Z

src/NuGetGallery/Services/PackageService.cs

+                            }
+                            else
+                            {
+                                _trace.Error(String.Format("UpdateIsLatestAsync retry exceeded for package '{0}'", packageRegistration.Id));


This case is really bad, right? What will the playbook be if we encounter one of these exceptions?

We'd hit this block if the request detected 3 concurrency conflicts in a row, in which case it would not update the flags.

This is unlikely to be a problem, since the real package CUD was already committed in a separate request. More than likely a concurrent request has already updated isLatest/isLatestStable, and the retry would detect that no changes are necessary.

This trace would at least give us monitoring if there are multiple concurrency conflicts in a row. Playbook would be to just check the package to see if the latest is set correctly... but more than likely it corrected itself.

joelverhagen · 2017-02-10T02:15:28Z

I'm interested in what a functional test on this change would look like. Before the change, could we have written a test to consistently reproduce the issue (by mocking out as much as possible and throwing enough threads at it)?

skofman1 · 2017-02-10T02:17:14Z

src/NuGetGallery/Controllers/ApiController.cs

@@ -393,6 +393,9 @@ public virtual Task<ActionResult> CreatePackagePost()
                            throw;
                        }

+                        // handle in separate transaction because of concurrency check with retry


nit: capital letter in the beginning of the comment to match the rest of the file

skofman1 · 2017-02-10T06:03:37Z

src/NuGetGallery/Services/PackageService.cs

+
+        protected internal virtual IEntitiesContext CreateNewEntitiesContext()
+        {
+            var connectionString = _entitiesContext.GetDatabase().Connection.ConnectionString;


we use the connection string from web.config: https://github.com/NuGet/NuGetGallery/blob/master/src/NuGetGallery.Core/Entities/EntitiesContext.cs#L25

return new EntitiesContext() should do the trick

skofman1 · 2017-02-10T06:10:43Z

src/NuGetGallery/Services/PackageService.cs

-                pv.IsLatest = false;
-                pv.IsLatestStable = false;
-                pv.LastUpdated = DateTime.UtcNow;
+                packageClears.Add(UpdateIsLatestPackageEdit.Set(pv, false, false));


please use named parameters, for readability

skofman1 · 2017-02-10T06:13:49Z

                // a release version and set it to IsLatest.

isLatestStable.

Refers to: src/NuGetGallery/Services/PackageService.cs:835 in 7f861cd. [](commit_id = 7f861cd, deletion_comment = False)

chenriksson · 2017-02-10T17:01:48Z

src/NuGetGallery/Services/PackageService.cs

+            if ((retryCount == 0) && (clears != null))
+            {
+                MergeUpdateIsLatestClearsWithSets(clears, sets);
+            }


Need to remove conditional around MergeUpdateIsLatestClearsWithSets. Originally I had bug where retry was calling back into UpdateIsLatestInDatabaseAsync without refresh and recalculating the latest.

cristinamanum · 2017-02-10T17:08:02Z

src/NuGetGallery/Services/PackageService.cs

@@ -779,5 +898,40 @@ private void NotifyIndexingService()
                }
            }
        }
+
+        private class UpdateIsLatestPackageEdit


It would be great to add few comments about this class.

cristinamanum · 2017-02-10T17:28:37Z

src/NuGetGallery/Services/PackageService.cs

+            return database.ExecuteSqlCommandAsync(sql, parameters);
+        }
+
+        private async Task<int> UpdateIsLatestInDatabaseWithConcurrencyCheckAsync(Database database, UpdateIsLatestPackageEdit packageEdit)


nit: line too long.

cristinamanum · 2017-02-10T17:30:20Z

src/NuGetGallery/Services/PackageService.cs

+            packageEdit.Package.Key, packageEdit.Package.IsLatest, packageEdit.Package.IsLatestStable, packageEdit.OriginalIsLatest, packageEdit.OriginalIsLatestStable);
+        }
+
+        private static bool PackagesMatch(Package first, Package second)


Maybe PackagesVersionMatch instead of the PackagesMatch?

cristinamanum · 2017-02-10T17:32:44Z

src/NuGetGallery/Services/PackageService.cs

+            }
+        }
+
+        private static void MergeUpdateIsLatestClearsWithSets(List<UpdateIsLatestPackageEdit> clears, List<UpdateIsLatestPackageEdit> sets)


It would be great to have few comments. Also the line length seems to be large.

skofman1 · 2017-02-10T17:52:17Z

src/NuGetGallery/Services/PackageService.cs

+            return database.ExecuteSqlCommandAsync(sql, parameters);
+        }
+
+        private async Task<int> UpdateIsLatestInDatabaseWithConcurrencyCheckAsync(Database database, UpdateIsLatestPackageEdit packageEdit)


int [](start = 27, length = 3)

returning true/false will make the code more readable.

skofman1 · 2017-02-10T17:54:07Z

src/NuGetGallery/Services/PackageService.cs

+
+                            if (retryCount++ < UpdateIsLatestMaxRetries)
+                            {
+                                await Task.Delay(retryCount * 500);


500 [](start = 62, length = 3)

const

skofman1 · 2017-02-10T18:01:01Z

src/NuGetGallery/Services/PackageService.cs

+                            if (retryCount++ < UpdateIsLatestMaxRetries)
+                            {
+                                await Task.Delay(retryCount * 500);
+                                await UpdateIsLatestAsync(packageRegistration, retryCount);


UpdateIsLatestAsync [](start = 38, length = 19)

this code could be simpler if the retries were done in UpdateIsLatestAsync. In this case UpdateIsLatestInDatabaseAsync calls the method that called it (UpdateIsLatestAsync), that calls it again, creating a recursion..
If UpdateIsLatestInDatabaseAsync returned a failure, UpdateIsLatestAsync can call it again until retries are exhausted.

skofman1 · 2017-02-10T18:26:30Z

        public async Task WillUpdateIsLatest1()

This naming is extremely confusing. Could you rename those?

Refers to: tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs:1074 in 7f861cd. [](commit_id = 7f861cd, deletion_comment = False)

skofman1 · 2017-02-10T18:35:25Z

Regarding functional tests: Scott wrote a test that pushes a bunch of packages and verified timestamps:

NuGetGallery/tests/NuGetGallery.FunctionalTests/ODataFeeds/V2FeedExtendedTests.cs

Line 64 in ce6c5d6

public async Task PackagesAppearInFeedInOrderTest()

You could do something similar

cristinamanum · 2017-02-10T19:12:45Z

src/NuGetGallery/Services/PackageService.cs

+        }
+
+        private Task UpdateIsLatestAsync(PackageRegistration packageRegistration, int retryCount)
+        {


Is the retryCount used in this method?

Yes, on retry of concurrency conflict in UpdateIsLatestInDatabaseAsync

scottbommarito · 2017-02-10T01:30:43Z

src/NuGetGallery/Services/PackageService.cs

+
+        private static bool PackagesMatch(Package first, Package second)
+        {
+            if (first == null)


Might be more concise and readable if you changed this to...

if (first == null || second == null) { return first != second; } return first.Version.Equals(second.Version, StringComparison.OrdinalIgnoreCase);

scottbommarito · 2017-02-10T01:53:54Z

src/NuGetGallery/Services/PackageService.cs

+
+            public Package Package { get; private set; }
+
+            public bool OriginalIsLatest { get; set; }


This and OriginalIsLatestStable should have a private set because they should not be changed after being initialized.

These may change in MergeUpdateIsLatestClearsWithSets

scottbommarito · 2017-02-10T01:55:07Z

src/NuGetGallery/Services/PackageService.cs

@@ -678,28 +685,149 @@ private void ValidatePackageTitle(PackageMetadata packageMetadata)
            }
        }

-        public async Task UpdateIsLatestAsync(PackageRegistration packageRegistration, bool commitChanges = true)
+        protected internal virtual Task<int> ExecuteSqlCommandAsync(Database database, string sql, params object[] parameters)


Can we merge this function with UpdateIsLatestInDatabaseWithConcurrencyCheckAsync? The code should fit inside one function just fine since we don't use it anywhere else.

scottbommarito · 2017-02-10T18:24:53Z

src/NuGetGallery/Controllers/ApiController.cs

@@ -470,6 +473,13 @@ private static ActionResult BadRequestForExceptionMessage(Exception ex)
            }

            await PackageService.MarkPackageUnlistedAsync(package);
+
+            // handle in separate transaction because of concurrency check with retry


Capitalize this comment just like you did this one and also add a period to the end (if the rest of the file does that).

scottbommarito · 2017-02-10T18:28:09Z

src/NuGetGallery/Controllers/ApiController.cs

@@ -470,6 +473,13 @@ private static ActionResult BadRequestForExceptionMessage(Exception ex)
            }

            await PackageService.MarkPackageUnlistedAsync(package);
+
+            // handle in separate transaction because of concurrency check with retry
+            if (package.IsLatest || package.IsLatestStable)


I would suggest putting UpdateIsLatestAsync back inside MarkPackageListedAsync but after the commit. You always call the two together.

See comment for MarkPackageUnlistedAsync. While this method also supports not committing changes, only test code does it. So, I could remove that option and move UpdateIsLatestAsync back inside, but I'm inclined to keep these methods consistent. Let me know if you disagree.

scottbommarito · 2017-02-10T18:38:25Z

src/NuGetGallery/Services/IPackageService.cs

+        /// <summary>
+        /// Updates IsLatest/IsLatestStable flags after a package CUD operation.
+        /// 
+        /// Database updates are applied on a separate context to better control refresh of entities when


I think it would be better if this comment were more specific. We want better control BECAUSE we were having issues with multiple requests operating on the isLatest fields of the package simultaneously and leading to multiple packages being marked as the latest or the latest prerelease.

scottbommarito · 2017-02-10T19:08:36Z

src/NuGetGallery/Services/PackageService.cs

            {
                return;
            }

-            // TODO: improve setting the latest bit; this is horrible. Trigger maybe?
+            // performed updates on separate context in order to refresh entities between retries


Capitalize please.

scottbommarito · 2017-02-10T19:10:52Z

src/NuGetGallery/Services/PackageService.cs

+            {
+                EntitiesConfiguration.SuspendExecutionStrategy = false;
+
+                var disposableContext = altEntitiesContext as IDisposable;


Could we move the declaration of altEntitiesContext inside the try-finally and then use a using statement instead of having this in the finally statement?

Yes, and you don't need to cast. DbContext implements IDisposable.

This was to support the Mock object used by tests, which wasn't IDisposable. Will see if there's another way.

scottbommarito · 2017-02-10T19:24:16Z

tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs

-                packageRepository.Verify(r => r.CommitChangesAsync(), Times.Never());
-            }
-
-            [Fact]
            public async Task CommitIfCommitChangesIsTrue()


I suggest renaming this to AlwaysCommitsChanges because the commit changes option was removed from the function.

scottbommarito · 2017-02-10T19:27:55Z

tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs

+            }
+
+            [Fact]
+            public async Task WillUpdateIsLatest3()


We should add some unit tests that test the behavior when transactions fail, and enumerate different possible scenarios. You can do this by mocking the UpdateIsLatestInDatabaseWithConcurrencyCheckAsync method and performing different behaviors based on the UpdateIsLatestPackageEdit object supplied.

skofman1 · 2017-02-23T16:15:21Z

src/NuGetGallery.Core/Entities/EntitiesConfiguration.cs

+            return new ExecutionStrategySuspension();
+        }
+
+        private class ExecutionStrategySuspension : IDisposable


RetriableExecutionStrategySuspension

skofman1 · 2017-02-23T16:46:13Z

Looks great!

dtivel · 2017-02-23T16:38:39Z

src/NuGetGallery.Core/Entities/EntitiesConfiguration.cs

        {
            get
            {
-                return (bool?)CallContext.LogicalGetData("SuspendExecutionStrategy") ?? false;
+                return (bool?)CallContext.LogicalGetData("UseRetriableExecutionStrategy") ?? false;


You can use nameof(UseRetriableExecutionStrategy).

Also, as part of the logic inversion, the fallback value here should be true not false.

dtivel · 2017-02-23T17:01:51Z

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

@@ -581,6 +581,8 @@ public class TheEditMethod
                    .Throws(new Exception("Shouldn't be called"));
                packageService.Setup(svc => svc.FindPackageByIdAndVersion("Foo", "1.0", true))
                    .Returns(package).Verifiable();
+                packageService.Setup(svc => svc.UpdateIsLatestAsync(It.IsAny<PackageRegistration>()))
+                    .Returns(Task.FromResult(0)).Verifiable();


Does Task.CompletedTask also work? I think it's more clear than Task.FromResult(0).

dtivel · 2017-02-23T17:02:16Z

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

@@ -1607,6 +1609,8 @@ public class TheVerifyPackageActionForPostRequests
                        .Returns(Task.CompletedTask);
                    fakePackageService.Setup(x => x.MarkPackageUnlistedAsync(fakePackage, false))
                        .Returns(Task.CompletedTask);
+                    fakePackageService.Setup(x => x.UpdateIsLatestAsync(It.IsAny<PackageRegistration>()))
+                        .Returns(Task.FromResult(0));


Task.CompletedTask?

dtivel · 2017-02-23T17:05:17Z

tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs

+            }
+
+            [Fact]
+            public async Task UpdateIsLatestAsync_SameLatestAndLatestStableVersionsWithClear()


SameNewLatestAndLatestStableVersions sounds more clear than SameLatestAndLatestStableVersionsWithClear.

chenriksson · 2017-02-24T15:10:16Z

@scottbommarito look ok to you?

dtivel · 2017-02-24T15:26:45Z

tests/NuGetGallery.FunctionalTests.Core/Helpers/ODataHelper.cs

-                WriteLine($"Package data does not contain '{propertyName}' timestamp!");
-                return null;
-            }
+                var propertiesStart = Math.Max(packageResponse.IndexOf("<m:properties>"), 0);


Why not just use XmlDocument or XDocument to read the XML?

Note that using XmlDocument or XDocument would also remove the potential for the regex to take an extremely long time as @blowdart noted.

blowdart · 2017-02-24T15:29:42Z

tests/NuGetGallery.FunctionalTests.Core/Helpers/ODataHelper.cs


-            var timestampStartIndex = timestampTagIndex + timestampStartTag.Length;
-            var timestampLength = packageResponse.Substring(timestampStartIndex).IndexOf(timestampEndTag);
+                    var propertyRegEx = new Regex(GetPackagePropertyRegexPattern);


Ensure there's a timeout on all regexs

scottbommarito

a couple nitpicks

scottbommarito · 2017-02-24T18:20:44Z

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

@@ -51,9 +50,9 @@ public class PackagesControllerFacts
            if (uploadFileService == null)
            {
                uploadFileService = new Mock<IUploadFileService>();
-                uploadFileService.Setup(x => x.DeleteUploadFileAsync(It.IsAny<int>())).Returns(Task.FromResult(0));
+                uploadFileService.Setup(x => x.DeleteUploadFileAsync(It.IsAny<int>())).Returns(Task.CompletedTask);


thanks for doing this...so much cleaner

scottbommarito · 2017-02-24T18:21:44Z

tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs

                indexingService.Object,
                packageNamingConflictValidator,
                auditingService);

+            packageService.Setup(s => s.TryUpdateIsLatestInDatabase(It.IsAny<IEntitiesContext>()))
+                .Returns(Task.FromResult(true));


Task.CompletedTask?

I only switched to Task.CompletedTask for async methods w/o return values. Since TryUpdateIsLatestInDatabase returns Task<bool>, I want to return true to look like the db update was successful.

scottbommarito · 2017-02-24T18:30:41Z

tests/NuGetGallery.FunctionalTests.Core/Helpers/ODataHelper.cs

-                WriteLine($"Package data does not contain '{propertyName}' timestamp!");
-                return null;
-            }
+                var propertiesStart = Math.Max(packageResponse.IndexOf("<m:properties>"), 0);


Note that using XmlDocument or XDocument would also remove the potential for the regex to take an extremely long time as @blowdart noted.

scottbommarito · 2017-02-24T18:38:42Z

tests/NuGetGallery.FunctionalTests/ODataFeeds/V2FeedExtendedTests.cs

+            }
+            Task.WaitAll(concurrentTasks);
+
+            await CheckPackageLatestVersions(packageId, packageVersions, expectedLatest: "7.0.2-abc", expectedLatestStable: "7.0.1");


I would prefer that you had these values in a variable instead of copying them here.

var latestVersion = "7.0.2-abc" var latestStable = "7.0.1" var latestVersionHalf = "3.0.2-abc" var latestStableHalf = "3.0.1" var packageVersions = new List<string>() { "1.0.0-a", "1.0.0-b", "1.0.0", "1.0.1", "1.0.2-abc", "2.0.0-a", "2.0.0-b", "2.0.0", "2.0.1", "2.0.2-abc", "3.0.0-a", "3.0.0-b", "3.0.0", latestStableHalf, latestVersionHalf, "4.0.0-a", "4.0.0-b", "4.0.0", "4.0.1", "4.0.2-abc", "6.0.0-a", "6.0.0-b", "6.0.0", "6.0.1", "6.0.2-abc", "7.0.0-a", "7.0.0-b", "7.0.0", latestStable, latestVersion }; ... await CheckPackageLatestVersions(packageId, packageVersions, latestVersion, latestStable); ... // unlist last half and verify; ~1-2 concurrency conflicts seen in testing for (int i = concurrentTasks.Length - 1; i > concurrentTasks.IndexOf(latestVersionHalf); i--) ... await CheckPackageLatestVersions(packageId, packageVersions, latestVersionHalf, latestStableHalf); // unlist remaining and verify; ~1-2 concurrency conflicts seen in testing for (int i = concurrentTasks.IndexOf(latestVersionHalf); i >= 0; i--)

scottbommarito · 2017-02-24T18:45:21Z

tests/NuGetGallery.FunctionalTests/ODataFeeds/V2FeedExtendedTests.cs

        private async Task CheckPackageTimestampsInOrder(List<string> packageIds, string timestampPropertyName,
-            DateTime operationStartTimestamp)
+            DateTime operationStartTimestamp, bool packagesListed)


I like that you're adding this to my test as well as yours, but this parameter is unnecessary because you can determine whether or not the packages are listed based on the feed itself. If the package has a Published property that is greater than DateTime.MinValue (pretty sure it's January 1st 1900) then it is listed. You can parse this and then determine whether or not it should be latest or latest stable. I tried to do this initially but didn't realize that Published represented the listed status in the feed.

You could of course change this into a "expectListed" parameter that verifies that the listed status from the feed is identical to what we expect.

skofman1 · 2017-02-28T16:38:39Z

src/NuGetGallery/Services/PackageService.cs

@@ -795,6 +795,7 @@ protected internal virtual IEntitiesContext CreateNewEntitiesContext()
                int retryCount = 1;
                do
                {
+                    await Task.Delay(retryCount * 250);


to reduce the chance of retry collisions, I would recommend introducing randomization into the sleep duration

…dateIsLatest calls on same package (#3548)" This reverts commit 27ea9bc.

* Revert "UpdateIsLatest concurrent unlist fix (#3695)" This reverts commit 551fd86. * Revert "Fix concurrent push test by disabling search hijacking on feed (#3641)" This reverts commit 00fb3fb. * Revert "IsLatest Fix: wrong connection string passed to retry context (#3632)" This reverts commit 1b6b5b0. * Revert "Fix #2514 - Use optimistic concurrency to prevent multiple UpdateIsLatest calls on same package (#3548)" This reverts commit 27ea9bc.

dnfclas added the cla-already-signed label Feb 10, 2017

chenriksson requested review from joelverhagen, shishirx34, ryuyu, dtivel, cristinamanum, skofman1, scottbommarito and xavierdecoster February 10, 2017 00:31

joelverhagen reviewed Feb 10, 2017

View reviewed changes

skofman1 reviewed Feb 10, 2017

View reviewed changes

skofman1 requested changes Feb 10, 2017

View reviewed changes

skofman1 reviewed Feb 10, 2017

View reviewed changes

chenriksson commented Feb 10, 2017

View reviewed changes

cristinamanum reviewed Feb 10, 2017

View reviewed changes

skofman1 reviewed Feb 10, 2017

View reviewed changes

cristinamanum reviewed Feb 10, 2017

View reviewed changes

scottbommarito suggested changes Feb 10, 2017

View reviewed changes

skofman1 reviewed Feb 23, 2017

View reviewed changes

skofman1 approved these changes Feb 23, 2017

View reviewed changes

dtivel approved these changes Feb 23, 2017

View reviewed changes

dtivel reviewed Feb 24, 2017

View reviewed changes

blowdart reviewed Feb 24, 2017

View reviewed changes

scottbommarito approved these changes Feb 24, 2017

View reviewed changes

skofman1 reviewed Feb 28, 2017

View reviewed changes

chenriksson added 15 commits February 28, 2017 09:28

Fix 2514: IsLatest set to multiple versions

aebb94a

cleanup

0f910e7

Avoid entity refresh by handling DB updates on separate context

75c9142

Revert unnecessary changes

f40484e

Fix retry count

c83e0f1

PR feedback

720b483

PR feedback (2)

2f15ae7

PR feedback (3)

b68f4c4

Use EF change tracking

921b1df

Functional test for concurrent UpdateIsLatest calls

26e1e31

PR feedback (4)

baf3289

Fix bug from last push

1ed3f01

Bug fix and functional test PR feedback

f78a2b2

Randomize UpdateIsLatest retry delay

8a28650

Fix merge conflict from rebase

315179b

chenriksson force-pushed the chenriks/islatest-optcon branch from 6d787b1 to 315179b Compare February 28, 2017 17:48

chenriksson merged commit 27ea9bc into dev Feb 28, 2017

chenriksson deleted the chenriks/islatest-optcon branch February 28, 2017 19:12

chenriksson mentioned this pull request Mar 6, 2017

Fix for UpdateIsLatestAsync retry connection string #3632

Merged

chenriksson added a commit that referenced this pull request Mar 25, 2017

Revert "Fix #2514 - Use optimistic concurrency to prevent multiple Up…

1898f46

…dateIsLatest calls on same package (#3548)" This reverts commit 27ea9bc.


		var packageRegistration = sets.First().Package.PackageRegistration;

		if (retryCount++ < UpdateIsLatestMaxRetries)


		public Package Package { get; private set; }

		public bool OriginalIsLatest { get; set; }

Fix #2514 - Use optimistic concurrency to prevent multiple UpdateIsLatest calls on same package #3548

Fix #2514 - Use optimistic concurrency to prevent multiple UpdateIsLatest calls on same package #3548

Conversation

chenriksson commented Feb 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelverhagen commented Feb 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skofman1 commented Feb 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skofman1 commented Feb 10, 2017

skofman1 commented Feb 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottbommarito Feb 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skofman1 commented Feb 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenriksson commented Feb 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottbommarito left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenriksson Feb 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottbommarito Feb 10, 2017 •

edited

chenriksson Feb 24, 2017 •

edited