Skip to content

Commit

Permalink
chore(rx-exorcize): Remove some more RX usage (#3707)
Browse files Browse the repository at this point in the history
I was in the neighborhood and took a short detour removing `observable` from the `BakeryService`

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
marchello2000 and mergify[bot] committed Jun 1, 2020
1 parent ef080b1 commit 11937c0
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.netflix.spinnaker.orca.bakery.api
import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.orca.bakery.api.manifests.BakeManifestRequest
import retrofit.http.*
import rx.Observable


/**
Expand All @@ -31,20 +30,20 @@ interface BakeryService {
Artifact bakeManifest(@Path("type") String type, @Body BakeManifestRequest bake)

@POST("/api/v1/{region}/bake")
Observable<BakeStatus> createBake(@Path("region") String region, @Body BakeRequest bake, @Query("rebake") String rebake)
BakeStatus createBake(@Path("region") String region, @Body BakeRequest bake, @Query("rebake") String rebake)

@GET("/api/v1/{region}/status/{statusId}")
Observable<BakeStatus> lookupStatus(@Path("region") String region, @Path("statusId") String statusId)
BakeStatus lookupStatus(@Path("region") String region, @Path("statusId") String statusId)

@GET("/api/v1/{region}/bake/{bakeId}")
Observable<Bake> lookupBake(@Path("region") String region, @Path("bakeId") String bakeId)
Bake lookupBake(@Path("region") String region, @Path("bakeId") String bakeId)

//
// Methods below this line are not supported by the Netflix Bakery, and are only available
// iff bakery.roscoApisEnabled is true.
//

@GET("/bakeOptions/{cloudProvider}/baseImages/{imageId}")
Observable<BaseImage> getBaseImage(@Path("cloudProvider") String cloudProvider,
BaseImage getBaseImage(@Path("cloudProvider") String cloudProvider,
@Path("imageId") String imageId)
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class CompletedBakeTask implements Task {
TaskResult execute(@Nonnull StageExecution stage) {
def bakery = bakerySelector.select(stage)
def bakeStatus = stage.context.status as BakeStatus
def bake = bakery.service.lookupBake(stage.context.region as String, bakeStatus.resourceId).toBlocking().first()
def bake = bakery.service.lookupBake(stage.context.region as String, bakeStatus.resourceId)
// This treatment of ami allows both the aws and gce bake results to be propagated.
def results = [
ami: bake.ami ?: bake.imageName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class CreateBakeTask implements RetryableTask {
// throw a Retrofit exception (HTTP 404 Not Found)
def bake = bakeFromContext(stage, bakery)
String rebake = shouldRebake(stage) ? "1" : null
def bakeStatus = bakery.service.createBake(stage.context.region as String, bake, rebake).toBlocking().single()
def bakeStatus = bakery.service.createBake(stage.context.region as String, bake, rebake)

def stageOutputs = [
status : bakeStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class MonitorBakeTask implements OverridableTimeoutRetryableTask {

try {
def bakery = bakerySelector.select(stage)
def newStatus = bakery.service.lookupStatus(region, previousStatus.id).toBlocking().single()
def newStatus = bakery.service.lookupStatus(region, previousStatus.id)
if (isCanceled(newStatus.state) && previousStatus.state == BakeStatus.State.PENDING) {
log.info("Original bake was 'canceled', re-baking (executionId: ${stage.execution.id}, previousStatus: ${previousStatus.state})")
def rebakeResult = createBakeTask.execute(stage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import com.netflix.spinnaker.orca.bakery.config.BakeryConfigurationProperties
import retrofit.http.Body
import retrofit.http.Path
import retrofit.http.Query
import rx.Observable
import spock.lang.Specification
import spock.lang.Subject
import spock.lang.Unroll
Expand Down Expand Up @@ -132,22 +131,22 @@ class BakerySelectorSpec extends Specification {
}

@Override
Observable<BakeStatus> createBake(@Path("region") String region, @Body BakeRequest bake, @Query("rebake") String rebake) {
BakeStatus createBake(@Path("region") String region, @Body BakeRequest bake, @Query("rebake") String rebake) {
return null
}

@Override
Observable<BakeStatus> lookupStatus(@Path("region") String region, @Path("statusId") String statusId) {
BakeStatus lookupStatus(@Path("region") String region, @Path("statusId") String statusId) {
return null
}

@Override
Observable<Bake> lookupBake(@Path("region") String region, @Path("bakeId") String bakeId) {
Bake lookupBake(@Path("region") String region, @Path("bakeId") String bakeId) {
return null
}

@Override
Observable<BaseImage> getBaseImage(@Path("cloudProvider") String cloudProvider, @Path("imageId") String imageId) {
BaseImage getBaseImage(@Path("cloudProvider") String cloudProvider, @Path("imageId") String imageId) {
return null
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class BakeryServiceSpec extends Specification {
)

expect:
with(bakery.lookupStatus(region, statusId).toBlocking().first()) {
with(bakery.lookupStatus(region, statusId)) {
id == statusId
state == BakeStatus.State.COMPLETED
resourceId == bakeId
Expand All @@ -106,7 +106,7 @@ class BakeryServiceSpec extends Specification {
)

when:
bakery.lookupStatus(region, statusId).toBlocking().first()
bakery.lookupStatus(region, statusId)

then:
def ex = thrown(RetrofitError)
Expand Down Expand Up @@ -136,7 +136,7 @@ class BakeryServiceSpec extends Specification {
)

expect: "createBake should return the status of the bake"
with(bakery.createBake(region, bake, null).toBlocking().first()) {
with(bakery.createBake(region, bake, null)) {
id == statusId
state == BakeStatus.State.PENDING
resourceId == bakeId
Expand Down Expand Up @@ -175,7 +175,7 @@ class BakeryServiceSpec extends Specification {
)

expect: "createBake should return the status of the bake"
with(bakery.createBake(region, bake, null).toBlocking().first()) {
with(bakery.createBake(region, bake, null)) {
id == statusId
state == BakeStatus.State.RUNNING
resourceId == bakeId
Expand All @@ -202,7 +202,7 @@ class BakeryServiceSpec extends Specification {
)

expect:
with(bakery.lookupBake(region, bakeId).toBlocking().first()) {
with(bakery.lookupBake(region, bakeId)) {
id == bakeId
ami == "ami"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl
import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl
import retrofit.RetrofitError
import retrofit.client.Response
import rx.Observable
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject
Expand All @@ -50,7 +49,7 @@ class CompletedBakeTaskSpec extends Specification {
def "finds the AMI and artifact created by a bake"() {
given:
def bakery = Stub(BakeryService) {
lookupBake(region, bakeId) >> Observable.from(new Bake(id: bakeId, ami: ami, artifact: artifact))
lookupBake(region, bakeId) >> new Bake(id: bakeId, ami: ami, artifact: artifact)
}

task.bakerySelector = Mock(BakerySelector)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils
import retrofit.RetrofitError
import retrofit.client.Response
import retrofit.mime.TypedString
import rx.Observable
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject
Expand Down Expand Up @@ -238,7 +237,7 @@ class CreateBakeTaskSpec extends Specification {
task.execute(bakeStage)

then:
1 * bakery.createBake(bakeConfig.region, _ as BakeRequest, null) >> Observable.from(runningStatus)
1 * bakery.createBake(bakeConfig.region, _ as BakeRequest, null) >> runningStatus
}

def "should surface error message (if available) on a 404"() {
Expand Down Expand Up @@ -276,7 +275,7 @@ class CreateBakeTaskSpec extends Specification {
def bakery = Mock(BakeryService) {
1 * createBake(*_) >> {
bake = it[1]
Observable.from(runningStatus)
runningStatus
}
}
Expand Down Expand Up @@ -323,7 +322,7 @@ class CreateBakeTaskSpec extends Specification {
def bakery = Mock(BakeryService) {
1 * createBake(*_) >> {
bake = it[1]
Observable.from(runningStatus)
runningStatus
}
}
Expand Down Expand Up @@ -447,7 +446,7 @@ class CreateBakeTaskSpec extends Specification {
def "outputs the status of the bake"() {
given:
def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}
and:
Expand Down Expand Up @@ -477,7 +476,7 @@ class CreateBakeTaskSpec extends Specification {
def "outputs the packageName of the bake"() {
given:
def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}
and:
Expand Down Expand Up @@ -517,7 +516,7 @@ class CreateBakeTaskSpec extends Specification {
}
def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}
and:
Expand Down Expand Up @@ -568,7 +567,7 @@ class CreateBakeTaskSpec extends Specification {
}

def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}

and:
Expand Down Expand Up @@ -617,7 +616,7 @@ class CreateBakeTaskSpec extends Specification {
}

def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}

and:
Expand Down Expand Up @@ -666,7 +665,7 @@ class CreateBakeTaskSpec extends Specification {
}

def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}

and:
Expand Down Expand Up @@ -715,7 +714,7 @@ class CreateBakeTaskSpec extends Specification {
}

def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(runningStatus)
createBake(*_) >> runningStatus
}

and:
Expand Down Expand Up @@ -796,7 +795,7 @@ class CreateBakeTaskSpec extends Specification {
it.buildNumber == "69"
it.commitHash == null
},
null) >> Observable.from(runningStatus)
null) >> runningStatus

where:
triggerInfo | contextInfo
Expand Down Expand Up @@ -850,7 +849,7 @@ class CreateBakeTaskSpec extends Specification {
it.buildNumber == null &&
it.commitHash == null
},
null) >> Observable.from(runningStatus)
null) >> runningStatus

where:
triggerInfo | contextInfo
Expand Down Expand Up @@ -904,7 +903,7 @@ class CreateBakeTaskSpec extends Specification {
it.buildNumber == null &&
it.commitHash == null
},
null) >> Observable.from(runningStatus)
null) >> runningStatus

where:
triggerInfo | contextInfo
Expand All @@ -924,7 +923,7 @@ class CreateBakeTaskSpec extends Specification {
def bakery = Mock(BakeryService) {
1 * createBake(*_) >> {
bake = it[1]
Observable.from(runningStatus)
runningStatus
}
}

Expand Down Expand Up @@ -957,7 +956,7 @@ class CreateBakeTaskSpec extends Specification {
def "sets previouslyBaked flag to #previouslyBaked when status is #status.state"() {
given:
def bakery = Stub(BakeryService) {
createBake(*_) >> Observable.from(status)
createBake(*_) >> status
}

and:
Expand Down Expand Up @@ -1018,7 +1017,7 @@ class CreateBakeTaskSpec extends Specification {
it.baseLabel == "release" &&
it.baseOs == "ubuntu"
} as BakeRequest,
"1") >> Observable.from(runningStatus)
"1") >> runningStatus
0 * _
}

Expand Down Expand Up @@ -1057,7 +1056,7 @@ class CreateBakeTaskSpec extends Specification {
it.baseLabel == "release" &&
it.baseOs == "ubuntu"
} as BakeRequest,
queryParameter) >> Observable.from(runningStatus)
queryParameter) >> runningStatus
0 * _

when:
Expand All @@ -1075,7 +1074,7 @@ class CreateBakeTaskSpec extends Specification {
it.baseLabel == "release" &&
it.baseOs == "ubuntu"
} as BakeRequest,
null) >> Observable.from(runningStatus)
null) >> runningStatus
0 * _

where:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import com.netflix.spinnaker.orca.bakery.BakerySelector
import com.netflix.spinnaker.orca.bakery.api.BakeStatus
import com.netflix.spinnaker.orca.bakery.api.BakeryService
import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl
import rx.Observable
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject
Expand All @@ -45,7 +44,7 @@ class MonitorBakeTaskSpec extends Specification {
def previousStatus = new BakeStatus(id: id, state: BakeStatus.State.PENDING)
def stage = new StageExecutionImpl(pipeline, "bake", [region: "us-west-1", status: previousStatus])
def bakery = Mock(BakeryService) {
lookupStatus(stage.context.region, id) >> Observable.from(new BakeStatus(id: id, state: bakeState, result: bakeResult))
lookupStatus(stage.context.region, id) >> new BakeStatus(id: id, state: bakeState, result: bakeResult)
}

and:
Expand Down Expand Up @@ -84,9 +83,8 @@ class MonitorBakeTaskSpec extends Specification {
def previousStatus = new BakeStatus(id: id, state: BakeStatus.State.PENDING)
def stage = new StageExecutionImpl(pipeline, "bake", [region: "us-west-1", status: previousStatus])
def bakery = Mock(BakeryService) {
lookupStatus(stage.context.region, id) >> Observable.from(
new BakeStatus(id: id, state: state, result: null)
)
lookupStatus(stage.context.region, id) >>
new BakeStatus(id: id, state: state, result: null)
}

and:
Expand Down Expand Up @@ -124,7 +122,7 @@ class MonitorBakeTaskSpec extends Specification {
def previousStatus = new BakeStatus(id: id, state: BakeStatus.State.PENDING)
def stage = new StageExecutionImpl(pipeline, "bake", [region: "us-west-1", status: previousStatus])
def bakery = Mock(BakeryService) {
lookupStatus(stage.context.region, id) >> Observable.from(new BakeStatus(id: id, state: BakeStatus.State.COMPLETED))
lookupStatus(stage.context.region, id) >> new BakeStatus(id: id, state: BakeStatus.State.COMPLETED)
}

and:
Expand Down

0 comments on commit 11937c0

Please sign in to comment.