Updates to fix coercion errors. DSDEEPB-727 #84
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kshakir
force-pushed
the
ks_coercion_errors
branch
2 times, most recently
from
bbcd0cd to
e09e881
Compare
| @@ -39,42 +42,43 @@ class WorkflowManagerActorSpec extends CromwellTestkitSpec("WorkflowManagerActor | |||
|
|
|||
| val actual = outputs.map { case (k, WdlString(string)) => k -> string } | |||
| actual shouldEqual Map(HelloWorld.OutputKey -> HelloWorld.OutputValue) | |||
| Await.result(dataAccess.shutdown(), Duration.Inf) | |||
There was a problem hiding this comment.
could this use a loan pattern for the DataAccess? (Warning: did not test this)
with { dataAccess =>
doStuff()
}
def with[T](work: DataAccess => T): T = {
val dataAccess = DataAccess()
work(dataAccess)
Await.result(dataAccess.shutdown(), Duration.Inf)
}
There was a problem hiding this comment.
👍 for loan pattern
aside: Beyond thinking this talk from Dick Wall is worthwhile he does have an amusing anecdote about bioinformaticists abusing the loan pattern (just after 13:00 if you want to skip the rest)
kshakir
force-pushed
the
ks_coercion_errors
branch
from
e09e881 to
d062d8c
Compare
|
Ready for next round of review. Also, please let me know if I didn't address anything from the first round with either a sufficient fix or a comment. |
|
With the addition of some comments in the spots where there's discussion, looks good to me. 😄 |
Fixed Symbol doubly qualifying the symbol names. (thx mcovarr/scottfrazer) Added `WdlType.fromRawString`, with test against respective `WdlValue.toRawString`. `DummyDataAccess` replaced with using `DataAccess` instances, with cleanup of connections. When creating in memory databases will create unique `DataAccess` instances, just like Dummy. TestSlickDatabase now prints a warning, instead of an error, when unable to connect to MySql.
kshakir
force-pushed
the
ks_coercion_errors
branch
from
d062d8c to
751385c
Compare
|
👍 from me |
mcovarr
added a commit
that referenced
this pull request
Jul 8, 2015
Updates to fix coercion errors. DSDEEPB-727
mcovarr
pushed a commit
that referenced
this pull request
Oct 4, 2017
Prevent stack overflow for bad WDL
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed Symbol doubly qualifying the symbol names. (thx mcovarr/scottfrazer)
Added
WdlType.fromRawString, with test against respectiveWdlValue.toRawString.DummyDataAccessreplaced with usingDataAccessinstances, with cleanup of connections.When creating in memory databases will create unique
DataAccessinstances, just like Dummy.TestSlickDatabase now prints a warning, instead of an error, when unable to connect to MySql.