Skip to content
This repository was archived by the owner on Oct 29, 2023. It is now read-only.

Conversation

deflaux
Copy link
Contributor

@deflaux deflaux commented Jun 30, 2015

  1. Fixes Fix oauth flow to only occur once #95
  2. Depends on Facilitate creation of OfflineAuth via previously performed oauth flow. utils-java#33
  3. Using same appname for all dataflow-java pipelines with OfflineAuth created via GenomicsOptions.Methods.getGenomicsAuth so that we only have to do oauth once when trying a few different pipelines. (we have it this way in spark-examples too
  4. Removes now unnecessary flag for headless operation.

Tested via:

  • mvn verify
  • dataflow pipelines CountReads and VariantSimilarity kicked off from local machine
  • dataflow pipelines CountReads and VariantSimilarity kicked off from GCE (headless)

@iliat I think https://github.com/googlegenomics/dataflow-java/blob/master/src/main/java/com/google/cloud/genomics/dataflow/utils/GCSOptions.java is mostly obsolete now that Dataflow oauth flow includes the cloud platform scope which covers GCS and Google Genomics. Also, could use a rename - it doesn't add any new options, just makes it convenient to get a GCS client.

@deflaux
Copy link
Contributor Author

deflaux commented Jun 30, 2015

@jean-philippe-martin the build of utils-java is in maven central now and travis-ci is happy

@jean-philippe-martin
Copy link
Contributor

Excellent!

On Tue, Jun 30, 2015 at 3:51 PM, Nicole Deflaux notifications@github.com
wrote:

@jean-philippe-martin https://github.com/jean-philippe-martin the build
of utils-java is in maven central now and travis-ci is happy


Reply to this email directly or view it on GitHub
#104 (comment)
.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the current usage is that new apps will use or extend GenomicsOptions. But that would mean they all get the same app name, doesn't it?

It seems more reasonable to have the app write its own name somewhere. Either in the options as today, or we could just make it an argument go getGenomicsAuth if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, but in addition the change was not needed anyway since Dataflow's storage directory for the StoredCredential is the same for all pipelines unless the user changes it via --credentialDir. (Before when we were doing our own oauth flow, it was triggered for each different pipeline since appname was used in the directory path for the stored credential.)

I reverted that change.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, happy end then!

On Wed, Jul 1, 2015 at 1:36 PM, Nicole Deflaux notifications@github.com
wrote:

In
src/main/java/com/google/cloud/genomics/dataflow/utils/GenomicsOptions.java
#104 (comment)
:

@@ -33,28 +29,30 @@

  • annotated with a @JsonIgnore annotation.
    */
    public interface GenomicsOptions extends DataflowPipelineOptions {
    +
    • // Use the same appName for all pipelines in this collection so that the oauth flow only needs to happen once.
    • public static final String APP_NAME = "dataflow-java";

You are right, but in addition the change was not needed anyway since
Dataflow's storage directory for the StoredCredential is the same for all
pipelines unless the user changes it via --credentialDir. (Before when we
were doing our own oauth flow, it was triggered for each different pipeline
since appname was used in the directory path for the stored credential.)

I reverted that change.


Reply to this email directly or view it on GitHub
https://github.com/googlegenomics/dataflow-java/pull/104/files#r33721295
.

deflaux added 2 commits July 1, 2015 13:28
The change was not needed since Dataflow's storage directory for the StoredCredential is the same for all pipelines unless the user changes it via --credentialDir.
@jean-philippe-martin
Copy link
Contributor

LGTM (assuming tests pass)

deflaux added a commit that referenced this pull request Jul 1, 2015
Use Dataflow's oauth flow.
@deflaux deflaux merged commit ccdf917 into googlegenomics:master Jul 1, 2015
jiridanek pushed a commit to jiridanek/dataflow-java that referenced this pull request Jan 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants