Skip to content

Conversation

BenFradet
Copy link
Contributor

This is the sister PR to #1216.

I also removed the pubsubOptions param from newChannel since it doesn't seem to be used.

I'm keen on writing unit tests for PubSubAdmin next if possible.

@@ -19,7 +19,7 @@ package com.spotify.scio.pubsub

import com.google.pubsub.v1.PublisherGrpc.PublisherBlockingStub
import com.google.pubsub.v1.SubscriberGrpc.SubscriberBlockingStub
import com.google.pubsub.v1.{GetTopicRequest, PublisherGrpc, SubscriberGrpc, Subscription, Topic}
import com.google.pubsub.v1._
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BenFradet can you keep the expanded version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure 👍

Copy link
Contributor

@regadas regadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx! @BenFradet

@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #1483 into master will decrease coverage by 10.48%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1483       +/-   ##
===========================================
- Coverage   78.92%   68.44%   -10.49%     
===========================================
  Files         170      170               
  Lines        5153     5156        +3     
  Branches      317      396       +79     
===========================================
- Hits         4067     3529      -538     
- Misses       1086     1627      +541
Impacted Files Coverage Δ
...in/scala/com/spotify/scio/pubsub/PubSubAdmin.scala 0% <0%> (ø) ⬆️
.../scala/com/spotify/scio/bigquery/client/Jobs.scala 0% <0%> (-100%) ⬇️
.../spotify/scio/bigquery/BigQueryPartitionUtil.scala 0% <0%> (-100%) ⬇️
...ala/com/spotify/scio/bigquery/client/LoadOps.scala 0% <0%> (-100%) ⬇️
.../com/spotify/scio/bigquery/client/ExtractOps.scala 0% <0%> (-100%) ⬇️
...scala/com/spotify/scio/bigquery/MockBigQuery.scala 0% <0%> (-94.74%) ⬇️
...la/com/spotify/scio/bigquery/dynamic/package.scala 0% <0%> (-94.12%) ⬇️
...scala/com/spotify/scio/bigquery/client/Cache.scala 0% <0%> (-94.12%) ⬇️
...potify/scio/runners/dataflow/DataflowContext.scala 0% <0%> (-92.86%) ⬇️
...cala/com/spotify/scio/bigquery/client/JobOps.scala 0% <0%> (-90.7%) ⬇️
... and 24 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 073959c...cdde7db. Read the comment docs.

@regadas regadas merged commit 8410cb0 into spotify:master Oct 23, 2018
@BenFradet BenFradet deleted the feature/sub-exists branch October 23, 2018 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants