bahir-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ire7715 <...@git.apache.org>
Subject [GitHub] bahir issue #48: [BAHIR-122] [PubSub] Make "ServiceAccountCredentials" reall...
Date Tue, 25 Jul 2017 04:12:49 GMT
Github user ire7715 commented on the issue:

    https://github.com/apache/bahir/pull/48
  
    Thanks @ckadner for suggestion.
    Agree with your point, just modified the `SparkGCPCredentialsBuilderSuite` and make it
read the environment variables for keyfiles defined in `PubsubTestUtils`. It ignores the test
cases if the keyfiles or email account are not set(or file doesn't exist) and shows the hint
message.
    Also discovered that `ServiceAccountCredentials.getFileBuffer` didn't check file existence
before opening, just added the check.
    
    Don't know if the force push would bother you when reviewing, so I made some manual-diff:
    
    ----
    
    SparkGCPCredentials.scala L66
    \- `if (filePath.isEmpty) Array[Byte]()`
    \+ `if (filePath.isEmpty || !Files.exists(Paths.get(filePath.get))) Array[Byte]()`
    
    ----
    
    SparkGCPCredentialsBuilderSuite.scala
    > Deleted resource variables, let `jsonFilePath`, `p12FilePath` and `emailAccount`
read system environment.
    > Implemented `jsonAssumption` and `p12Assumption`, to check the existence of corresponding
environment variables and files.
    > No longer expected `credential.refreshToken` thrown an Exception, expected it could
retrieve the real token.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message