Request Boundary methods - beginRequest()/endRequest() implementation #708

ulvii · 2018-05-30T16:18:02Z

Implementation and a test for Request Boundary methods that were introduced with JAVA 9.

JDBC spec:
https://docs.oracle.com/javase/9/docs/api/java/sql/Connection.html#beginRequest--
https://docs.oracle.com/javase/9/docs/api/java/sql/Connection.html#endRequest--

beginRequest():

Backs up the following connection properties that are modifiable through public methods - databaseAutoCommitMode, transactionIsolationLevel, networkTimeout, holdability, sendTimeAsDatetime, statementPoolingCacheSize, disableStatementPooling, serverPreparedStatementDiscardThreshold, enablePrepareOnFirstPreparedStatementCall, sCatalog, sqlWarnings.
Marks the beginning of the work unit.
No-op if a work unit has already started.

endRequest():

Rolls back the open transactions.
Clears/closes the statements created inside the work unit.
Reverts the modifiable connection properties mentioned above back to their original values and calls their setters.
Marks the end of the work unit.
No-op without an intervening call to beginRequest().

Remarks:
The the methods do not keep track of columnEncryptionTrustedMasterKeyPaths, because this is a static field and shared among the connections.

…tation

codecov-io · 2018-05-30T16:36:57Z

Codecov Report

Merging #708 into dev will increase coverage by 0.19%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##                dev     #708      +/-   ##
============================================
+ Coverage     48.14%   48.33%   +0.19%     
- Complexity     2579     2618      +39     
============================================
  Files           113      113              
  Lines         26553    26625      +72     
  Branches       4457     4480      +23     
============================================
+ Hits          12783    12869      +86     
+ Misses        11629    11603      -26     
- Partials       2141     2153      +12

Flag	Coverage Δ	Complexity Δ
#JDBC42	`47.77% <3.89%> (-0.21%)`	`2568 <1> (-2)`
#JDBC43	`48.24% <85.71%> (+0.27%)`	`2614 <21> (+40)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java	`59.5% <100%> (+0.13%)`	`135 <0> (ø)`	⬇️
...icrosoft/sqlserver/jdbc/SQLServerConnection43.java	`100% <100%> (ø)`	`7 <2> (+2)`	⬆️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java	`47.98% <84.05%> (+2.22%)`	`321 <19> (+33)`	⬆️
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java	`60.91% <0%> (-0.66%)`	`88% <0%> (-1%)`
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java	`55.05% <0%> (-0.6%)`	`0% <0%> (ø)`
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java	`44.96% <0%> (-0.45%)`	`107% <0%> (-1%)`
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java	`62.47% <0%> (-0.21%)`	`63% <0%> (ø)`
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java	`44.94% <0%> (ø)`	`16% <0%> (ø)`	⬇️
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java	`52.42% <0%> (ø)`	`239% <0%> (ø)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15a01c9...b215898. Read the comment docs.

rene-ye · 2018-06-05T00:12:37Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+                originalEnablePrepareOnFirstPreparedStatementCall = enablePrepareOnFirstPreparedStatementCall;
+                originalSCatalog = sCatalog;
+                originalSqlWarnings = sqlWarnings;
+                openStatements = new ArrayList<Statement>();


There's a lot more adding/removing compared to getting/setting. Using a LinkedList would yield better performance? The try-with-resources list removal behavior will still remain.

cheenamalhotra · 2018-06-05T03:18:47Z

src/test/java/com/microsoft/sqlserver/jdbc/requestboundary/RequestBoundaryMethodsTest.java

+            }
+            if (null != con) {
+                con.close();
+            }


Please use try-with-resources in all tests.

The method tests whether statements are closed after endRequest(). This should be fine.

I think if you're testing statments closed or not, you can use asserts after endRequest is called.
Assert.assertTrue(closeable.isClosed());

And connecion is not part of test, so it should be always in try-with-resources block. Basically, any closeable object created in tests must always be created in try-with-resources blocks to avoid resource leaks if tests fail due to any reason.

Asserts are already there.

I agree that we should use try-with-resources when possible, but for this scenario I will keep the finally block instead, because the method tests multiple statements between beginRequest()/ endRequest() and we would need to add multiple try blocks too. Which in my opinion will make the code less readable.

cheenamalhotra · 2018-06-05T03:28:52Z

src/test/java/com/microsoft/sqlserver/jdbc/requestboundary/RequestBoundaryMethodsTest.java

+                con.beginRequest();
+                thread1.start();
+                thread2.start();
+                thread3.start();


You could use CountDownLatch for triggering all threads at exact same time, I guess that is what we should be testing here.

No need to trigger threads at the same time, that's not the point of the test. We just need to make sure connection is back to its original state once the threads finished execution.

cheenamalhotra · 2018-06-05T03:34:21Z

src/test/java/com/microsoft/sqlserver/jdbc/requestboundary/RequestBoundaryMethodsTest.java

+                thread3.start();
+                try {
+                    // Wait for threads to complete
+                    Thread.sleep(3000);


Again, Thread.sleep is not required. Java Concurrency has CountDownLatch.await() or ExecutorService.awaitTermination() APIs that would wait for all threads to finish and then proceed further.

cheenamalhotra · 2018-06-05T03:43:39Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java

@@ -631,7 +631,9 @@ void closeInternal() {
        // Regardless what happens when cleaning up,
        // the statement is considered closed.
        assert !bIsClosed;
-
+
+        connection.removeOpenStatement(this);


This should be done right at the end of Statement.close() just before logging to avoid losing references of statements if they were not closed properly (failure possible in discardLastExecutionResults) due to any reason in a pooled connection scenario. It could lead into losing references/memory leakage of unclosed statements/resultsets.

cheenamalhotra · 2018-06-05T03:47:18Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+    private volatile SQLWarning originalSqlWarnings = null;
+    private List<Statement> openStatements = null;
+
+    protected void beginRequestInternal() throws SQLException {
        DriverJDBCVersion.checkSupportsJDBC43();


Please remove unnecessary checks in both internal methods - need not check for checksSupportsJDBC43 anymore since the public APIs will be available only with SQLServerConnection43 class object.

The reason why I added the check was to prevent 4.2 APIs from calling these methods. I guess we can remove it too.

cheenamalhotra · 2018-06-05T04:11:53Z

src/test/java/com/microsoft/sqlserver/jdbc/requestboundary/RequestBoundaryMethodsTest.java

+        boolean disableStatementPooling2 = false;
+        int serverPreparedStatementDiscardThreshold2 = 100;
+        boolean enablePrepareOnFirstPreparedStatementCall2 = true;
+        String sCatalog2 = "tempdb";


Why are we hardcoding database names (model & tempdb) here? This test would fail if attempted to run with Azure DB specified in connection String.

cheenamalhotra · 2018-06-05T04:34:01Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+                }
+                if (enablePrepareOnFirstPreparedStatementCall != originalEnablePrepareOnFirstPreparedStatementCall) {
+                    if (null == originalEnablePrepareOnFirstPreparedStatementCall) {
+                        setEnablePrepareOnFirstPreparedStatementCall(DEFAULT_ENABLE_PREPARE_ON_FIRST_PREPARED_STATEMENT_CALL);


Can we ignore false change by using getter directly for setting original value and reading new value as well?

Also why are we comparing null with original value - why do we care? The driver anyway calls getter() to retrive the property value.

Can we ignore false change by using getter directly for setting original value and reading new value as well? - Yes.

Also why are we comparing null with original value - why do we care? The driver anyway calls getter() to retrive the property value. - boolean cannot be null.

Well it's null by default, I guess that's why you're checking for null for original value, but if you always use getter() for original value - it will never be null and this check won't be needed. Also we don't even need to reset it explicitly to its default value - just store Original back if new value is different.

peterbae · 2018-06-07T21:47:07Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+                sqlWarnings = originalSqlWarnings;
+                if (null != openStatements) {
+                    while (!openStatements.isEmpty()) {
+                        try (Statement st = openStatements.get(0)) {


Check this logic again. This might result in infinite loop.

Actually, after discussing with Ulvi it turns out the try block calls close() which calls removeOpenStatement, so this while loop does eliminate elements from the list successfully. no change needed here.

peterbae · 2018-06-07T21:48:20Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+    private Boolean originalEnablePrepareOnFirstPreparedStatementCall;
+    private String originalSCatalog = null;
+    private volatile SQLWarning originalSqlWarnings = null;
+    private List<Statement> openStatements = null;


I don't think we need to assign false or null randomly to these variables - primitives like boolean will initialize to false by default, and objects will initialize to null by default. I'm not sure if this was intentional, but we could probably clean this up.

i like how you put the variables here (member variables usually go to the top, but I see that this class already contains instances where they put the relevant member variables near the relevant methods, so this is probably the right way to organize things. I'm assuming this was intentional)

peterbae · 2018-06-07T21:53:42Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+        synchronized (this) {
+            if (requestStarted) {
+                if (!databaseAutoCommitMode) {
+                    rollback();


Looks like you decided to do a rollback here. If database is not in autocommit mode, at least the rollback will always succeed. However, it looks like Oracle JDBC spec for endRequest suggests that if the pooling manager calls endRequest while there is a transaction going on, the driver throw a SQLException (https://docs.oracle.com/javase/9/docs/api/java/sql/Connection.html#endRequest--). Have you considered why we're not throwing an exception if there's a transaction?

Most of the 3rd party pool managers do a rollback before returning connections to the pool, I decided to keep it the same way.

cool, I think that's fair.

peterbae · 2018-06-07T22:21:46Z

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

+    private int originalStatementPoolingCacheSize;
+    private boolean originalDisableStatementPooling;
+    private int originalServerPreparedStatementDiscardThreshold;
+    private Boolean originalEnablePrepareOnFirstPreparedStatementCall;


oh also this is the wrapper Boolean, this would be null by default. Change this to "boolean" if you want to avoid weird confusions in the future.

enablePrepareOnFirstPreparedStatementCall is Boolean, kept the types same.

huh. you know what, I saw that getEnablePrepareOnFirstPreparedStatementCall() returns boolean and assumed enablePrepareOnFirstPreparedStatementCall was boolean too. Looking at the code, the Boolean was intentional (it makes use of the null state), so keeping the originalEnablePrepareOnFirstPreparedStatementCall as Boolean here is the right choice too.

* 623 fix * 623 change stash * Prepared Statement Caching fix for 'handle not found' errors * Fix for PS Caching issue - Calling reset instead of on type def changes * Updated comparison * Change back assert check. * Adding call to removeReference back + Fix for Batch processes intermittent failures. * Removed DBName and made changes to resetPrepStmtHandle method * Check for null handle before proceed * Adding Old Constrcutor back to AKV Implementation * Making baseURL final * Remove unnecessary code. * Use Bulk Copy API for batch insert operation * Parse bug fixing and test added * bug fix + additional tests * change reflection for testing * more test changes * Add parsing logic for -- comment * refactoring * Update snapshot * Bug fix / testing change * Reflect comment change * Feature | AKV Old Constructor changes - Reformatted code + Deprecated old Constructor and added a new constructor with 1 param * Mark computed columns as IS_GENERATEDCOLUMN in the result set returned by getColumns() (#695) * Fix | getColumns() API, changed column name from SS_IS_COMPUTED to IS_AUTOINCREMENT per JDBC specs | issue #600 * Fix | getColumns() API, changed column name from SS_IS_COMPUTED to IS_GENERATEDCOLUMN per JDBC specs | issue #600 * fix issue with redirection * Fix | PS Caching - Remove commented lines * Trigger Appveyor test * Fix | AKV Old Constructor - Calling the other constructor instead. * Fix | Reversed null checks * Resolving alignment problems and comments * Refactor two Bulk files into a common parent * javadoc changes * Applied formatter * fix problem with precision / scale * Fix | Fix some of the Javadoc warnings (#702) * fix issue with setting all to true * Resolved maven build warnings and java warnings regarding deprecated API (#701) * Resolving maven warnings * Removing jreVersion property Does not make sense now that we use final name in the build itself. Only used in 1 place, hard-coding java version for different builds as that's what it represents anyways. * java warnings * make bamoo fixes * resource bundle for junit test error strings (#698) resource bundle for error message strings in junit tests * undo some changes made to SQLServerConnection * apply resource bundling changes * Add support for JDK 10 in both Maven and Gradle (#691) * Feature | Added support for JDK 10 in both Maven and Gradle - builds jre10 jars for the driver, replacing jre9 * JDK 10 | Merge 42 classes to base classes to reduce class redundancy. * JDK 10 | Attempt to run JDK 10 with Appveyor * Remove unwanted space * Updating Travis script to use JDK 10 * Testing without addons * Update script for Jacoco report to build 43 profile * Revert driver changes for 42 compliance - to be added in a separate PR * Revert Test class changes for 42 compliance - to be done in a separate PR * Reformatted code * Add ID to jacoco plugin execution task * Kerberos Constrained Delegation Impersonated Credential Expiry fix (#636) fix for automatic credential discarding * update felix to 3.5.0 * Revised implementation Decided to not dispose user created credentials at all. * Updated flag set location * changes for 6.5.3 preview release * Revert "changes for 6.5.3 preview release" This reverts commit 5c6ccd3. * Changes in preparation for 6.5.3 preview release (#710) * changes for preview release * requested changes * jre version update changes * snapshot updates post release * remove on_dw, and remove redundant fmtonly * formatting * fix for getSchema when using "-" in name * Reformatting + adding more tests * inherit the connection property in statement + fix issue with null / empty string being passed in as values * Request Boundary methods - beginRequest()/endRequest() implementation (#708) * Add | Request Boundary Methods - beginRequest()/endRequest() implementation * Fix | Remove unused import from AbstractTest * Fix | Applying review comments * Fix | Moving RequestBoundaryMethodsTest.java to connection package * added error message in resource file and changed files accordingly * comment revisions * use TestResource * test changes removed finals removed database creation tracking * drop database before creating * replaced dropDBIfExists with Utils function * added try-with-resources nest avoid manually closing statements, and safetly handles resources. * Fixing logic / adding more tests * dont use test database in tests * Change exception handling as per JDBC specs * Add | Add missing license headers (#725) * remove some comments * Enable verify data (#724) Fix to enable data verification in Junit tests. Also addresses intermittent failures with Time/Timestamp where the precision was being inaccurately judged. * Fix | Refactored socket creation to simplify handling of socket creation Refactors socket creation in SocketFinder.findSocket(...) to simplify handling of socket creation. When the host resolves to a single address the driver now defers to getConnectedSocket(...) to create the socket without spawning any threads. This happens regardless of whether we're running on an IBM JDK. Previously the single address case would still use NIO on an IBM JDK. On non-IBM JDKs the driver now handles both IPv4 and IPv6 addresses concurrently with a single shared timeout. Previously hosts that resolved to both types of addresses were allowed half the timeout for socket creation per address type with the resolution performed sequentially. * reflect comments * Add support for UTF-8 feature extension. (#722) * Add | Support for UTF8 changes * changed how logger works, refactored code in SQLServerBulkCommon due to that, changed exception being thrown to BatchUpdateException, added same logic for parsing in executeLargeBatch, and added tests accordingly. * add more tests, make the prepared statement property go away * Feature | Introduce support for "Data Classification Specifications" on fetched resultsets (#709) * Feature | Data Classification Project | Phase 1 (contains temporary skipping 2 bytes) * Feature | Data Classification - Removing extra bytes added before * Feature | Data Classification - Added new test class for testing Data Classification support in the driver * Remove one println * Feature | Repackaged newly added files for Data Classification + improvements in source code * Feature | Changing tokens to bytes instead of int * Feature | Making variables private * Formatted code + dropTable method called from Utils * Feature | Data Classification - Changes as per review comments * Fix | Review comment changes * Change exception codes to follow series * Fix Conflict issue * Added missing Javadocs and headers for all new files * Added getter/setter public for the useBulkCopyForBatchInsert connection property. * Change implementation of child classes a bit * Remove dependencies from tests that are from outside required libraries * also remove hex from DBTable * Fix bamboo problem + refactor test code * Replace all connection and statements with try blocks * change spacing * refactor code * refactoring * Fix | Making driver default compliant to JDBC 4.2 Specs and update ADAL4J dependency to 1.6.0 (#711) * Feature | Added support for JDK 10 in both Maven and Gradle - builds jre10 jars for the driver, replacing jre9 * JDK 10 | Merge 42 classes to base classes to reduce class redundancy. * JDK 10 | Attempt to run JDK 10 with Appveyor * Remove unwanted space * Updating Travis script to use JDK 10 * Testing without addons * Update script for Jacoco report to build 43 profile * Minor fix in formatting to avoid conflicts * moving driver specific functions for SQLServerPreparedStatement * Remove unwanted code + Update Adal4J library dependency * changes for CallableStatement repeptitive delcarations * Remove an extra bracket due to conflict * changes for ISQLServerConnection there are problems with moving all Driver sepcific public methods. SQLServerConnectionPoolProxy also implements this interface and there are many public APIs (such as preparedstmt cacheing stuff) which it doesn't implement, and cannot be moved into the interface at this time. * lambda touch-up should generally stick to 1 line if possible. * changes for ISQLServerDataSource * updates for ISQLServerResultSet * Improvements | Missing interface APIs added + Code improvements * More changes for Interface missing methods * Implemented missing methods in SQLServerConnectionPoolProxy * Removed ISQLServerConnection43 for duplicated method definitions * Added APIs in interface for SQLServerResultSet * More cleanup done * Fix minor issues * Fix test failures and implement Serialization for HashKey * Fix JavaDoc errors and warnigs * More changes for CallableStatement APIs * More changes for Statement and Prepared Statament public APIs * Javadoc fix * More changes for SQL Server Bulk Record interface * Callable Statement missing APIs for Interface * Add missing desciptions * Reverting pom.xml change for this PR * Attempt to resolve conflicts * Remove Interface as not needed. * Added missing docs * Changes for Clob/Blob classes for compliance * Update ADAL4J with latest version * Changes for Data Source classes * Minor fixes to the new changes * Fix for failing tests * More changes for compliance * Add Javadocs and class headers * Fixed Malformed HTML Error in Javadocs * javadoc changes * more javadoc changes to make the abbreviations more clear * fix unchecked warning issue * Change HashKey in the driver to 256 Hash * Add Interface back to SQLServerConnection43 class * Revert "Change HashKey in the driver to 256 Hash" This reverts commit e6bef4e. * Changes for exceptions to throw SQLServerException type * 6.5.4 preview release changelog (#731) Release | Changelog for 6.5.4 preview release (#731) * Fix Conflict issues with master branch

ulvii added 14 commits May 29, 2018 12:56

Add | Request Boundary Methods - beginRequest()/endRequest() implemen…

943bcd2

…tation

Merge remote-tracking branch 'upstream/dev' into PoolingAPIs

e5bcec7

Fix | Remove unused import from AbstractTest

d4c8bde

Fix | Increase network timeout

0a569bc

Fix | Change catalog to model

00ead56

Fix | Wait for setNetworkTimeout to complete

ffe9b8e

Merge remote-tracking branch 'upstream/dev' into PoolingAPIs

972e93c

Fix | Change networkTimeout to milliseconds

8dd0082

Fix | Add newline to the end of Utils.java

20cfcad

Fix | Revert change to Utils.java

3cd8e66

Fix | Fix Utils.java

a76d8b1

Fix | Overwrite Utils from upstream

c48e849

Fix | Add the new methods back to Utils.java

7d695c9

Fix | Add new line to the end of RequestBoundaryMethodsTest.java

a93642e

cheenamalhotra added this to Under Peer Review in MSSQL JDBC Jun 1, 2018

cheenamalhotra added this to the 6.5.4 milestone Jun 1, 2018

rene-ye reviewed Jun 5, 2018

View reviewed changes

rene-ye previously approved these changes Jun 5, 2018

View reviewed changes

cheenamalhotra reviewed Jun 5, 2018

View reviewed changes

Fix | Applying review commits

9b3c32e

ulvii dismissed rene-ye’s stale review via 9b3c32e June 5, 2018 23:26

ulvii added 2 commits June 5, 2018 16:28

Fix | Revert changes to the SQLCollation

a7b76b8

Merge branch 'dev' into PoolingAPIs

5c01b32

ulvii added 3 commits June 5, 2018 17:20

Fix | 4.2 test fix

70f01a6

Merge remote-tracking branch 'origin/PoolingAPIs' into PoolingAPIs

59d43f2

Fix | Use master when dropping a database

950bf28

peterbae reviewed Jun 7, 2018

View reviewed changes

cheenamalhotra previously approved these changes Jun 7, 2018

View reviewed changes

Fix | Moving RequestBoundaryMethodsTest.java to connection package

b215898

ulvii dismissed cheenamalhotra’s stale review via b215898 June 7, 2018 23:43

peterbae approved these changes Jun 7, 2018

View reviewed changes

ulvii requested a review from lilgreenbird June 8, 2018 16:04

rene-ye approved these changes Jun 8, 2018

View reviewed changes

lilgreenbird approved these changes Jun 8, 2018

View reviewed changes

ulvii merged commit 5887439 into microsoft:dev Jun 8, 2018

MSSQL JDBC automation moved this from Under Peer Review to Closed/Merged PRs Jun 8, 2018

rene-ye mentioned this pull request Jun 18, 2018

Request Boundary methods - beginRequest()/endRequest() implementation… rene-ye/mssql-jdbc#44

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Request Boundary methods - beginRequest()/endRequest() implementation #708

Request Boundary methods - beginRequest()/endRequest() implementation #708

ulvii commented May 30, 2018

codecov-io commented May 30, 2018 •

edited

Loading

rene-ye Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018 •

edited

Loading

ulvii Jun 5, 2018

cheenamalhotra Jun 5, 2018 •

edited

Loading

ulvii Jun 5, 2018

peterbae Jun 7, 2018

peterbae Jun 7, 2018

peterbae Jun 7, 2018

peterbae Jun 7, 2018

peterbae Jun 7, 2018

ulvii Jun 7, 2018

peterbae Jun 7, 2018

peterbae Jun 7, 2018

ulvii Jun 7, 2018 •

edited

Loading

peterbae Jun 7, 2018

Request Boundary methods - beginRequest()/endRequest() implementation #708

Request Boundary methods - beginRequest()/endRequest() implementation #708

Conversation

ulvii commented May 30, 2018

codecov-io commented May 30, 2018 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

cheenamalhotra Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cheenamalhotra Jun 5, 2018 • edited Loading

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

ulvii Jun 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented May 30, 2018 •

edited

Loading

cheenamalhotra Jun 5, 2018 •

edited

Loading

cheenamalhotra Jun 5, 2018 •

edited

Loading

ulvii Jun 7, 2018 •

edited

Loading