add Jobs samples by averikitsch · Pull Request #1405 · GoogleCloudPlatform/python-docs-samples

averikitsch · 2018-03-15T19:47:50Z

No description provided.

andrewsg · 2018-03-15T19:51:00Z

dlp/jobs.py

+        type: (Optional) The type of job. Defaults to 'INSPECT'.
+            Choices:
+            DLP_JOB_TYPE_UNSPECIFIED
+            INSPECT_JOB: The job inspected Google Cloud for sensitive data.


Perhaps change this to "The job inspected content for sensitive data"

andrewsg · 2018-03-15T19:51:32Z

dlp/jobs.py

+        specified filter in the request.
+    Args:
+        project: The Google Cloud project id to use as a parent resource.
+        filter: (Optional) Filter expressions are made up of one or more


Do we have any further advice to give on what valid filters look like? It's okay if the answer is "no".

andrewsg · 2018-03-15T19:52:17Z

dlp/jobs.py

+    parent = dlp.project_path(project)
+
+    # If job type is specified, convert job type to number through enums.
+    from google.cloud.dlp_v2 import enums


I think we can do without this import and just replace enums with google.cloud.dlp.enums where they appear below.

andrewsg · 2018-03-15T19:53:37Z

dlp/jobs.py

+    # If job type is specified, convert job type to number through enums.
+    from google.cloud.dlp_v2 import enums
+    # Job type dictionary
+    job_type_to_int = {


The keys here (INSPECT, etc.) don't match the keys in the docstring above (INSPECT_JOB, etc) and it looks like they need to be exact matches to work. Let's change either the keys or the docstring.

andrewsg · 2018-03-15T19:55:03Z

dlp/jobs.py

+
+    # Iterate over results.
+    for job in response:
+        print('Job: %s; status: %s' % (job.name, job.JobState.Name(job.state)))


Can we use the .format() method of string replacement for consistency with other samples in this directory?

andrewsg · 2018-03-15T19:57:36Z

dlp/jobs.py

+    # Iterate over results.
+    for job in response:
+        print('Job: %s; status: %s' % (job.name, job.JobState.Name(job.state)))
+        info_type_stats = job.inspect_details.result.info_type_stats


I think we can do without listing the inspect_details results here; I suspect not all job types (risk analysis etc) will even have that attribute. The Node samples just show the job name, state, and the creation time.

lesv · 2018-03-15T21:37:06Z

dlp/jobs.py

+            DLP_JOB_TYPE_UNSPECIFIED
+            INSPECT_JOB: The job inspected Google Cloud for sensitive data.
+            RISK_ANALYSIS_JOB: The job executed a Risk Analysis computation.
+    Returns:


Newline before Returns:

lesv · 2018-03-15T21:38:27Z

dlp/jobs.py

+    Args:
+        project: The Google Cloud project id to use as a parent resource.
+        job_name: The name of the DlpJob resource to be deleted.
+    Returns:


lesv · 2018-03-15T21:39:42Z

dlp/jobs.py

+        'filter.')
+    list_parser.add_argument(
+        'project',
+        help='The Google Cloud project id to use as a parent resource.')


Consider striking The Google Cloud

lesv · 2018-03-15T21:40:02Z

dlp/jobs.py

+        help='Delete results of a Data Loss Prevention API job.')
+    delete_parser.add_argument(
+        'project',
+        help='The Google Cloud project id to use as a parent resource.')


lesv · 2018-03-15T21:42:28Z

dlp/jobs_test.py

+import pytest
+import os
+
+GCLOUD_PROJECT = os.getenv('GCLOUD_PROJECT')


I believe we prefer GOOGLE_CLOUD_PROJECT for both sides

I agree in principle, but unfortunately the other languages such as Node are still using GCLOUD_PROJECT; perhaps we should standardize across all languages at once rather than try to change it in this PR.

@jonparrott PTAL
@broady PTAL

We were supposed to standardized on GOOGLE_CLOUD_PROJECT over two years ago.

lesv · 2018-03-16T23:45:55Z

@averikitsch Can you merge, or do you need Andrew or I to do it?

lesv · 2018-03-16T23:46:45Z

@andrewsg @jonparrott This repo should really have kokoro or something checking on submit.

theacodes · 2018-03-16T23:52:14Z

It has travis. No idea why it's not running.

andrewsg · 2018-03-17T00:04:58Z

I don't think travis monitors prs into branches, does it? It'll run when we merge into master though.

…

On Fri, Mar 16, 2018, 5:00 PM Averi Kitsch ***@***.***> wrote: Merged #1405 <#1405>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1405 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAp517CN8tWVMAL8VTID56cQ3qxr4DLVks5tfFICgaJpZM4Ssxxt> .

theacodes · 2018-03-17T00:06:06Z

Oh right On Fri, Mar 16, 2018, 5:05 PM Andrew Gorcester <notifications@github.com> wrote:

…

I don't think travis monitors prs into branches, does it? It'll run when we merge into master though. On Fri, Mar 16, 2018, 5:00 PM Averi Kitsch ***@***.***> wrote: > Merged #1405 > <#1405>. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #1405 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AAp517CN8tWVMAL8VTID56cQ3qxr4DLVks5tfFICgaJpZM4Ssxxt > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1405 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc4tm3TjmY-NJERfxtghB0vv51VlEks5tfFMrgaJpZM4Ssxxt> .

job samples and tests

56c0797

averikitsch assigned andrewsg and lesv Mar 15, 2018

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 15, 2018

andrewsg requested changes Mar 15, 2018

View reviewed changes

lesv suggested changes Mar 15, 2018

View reviewed changes

averikitsch added 2 commits March 16, 2018 09:25

in response to PR

bbef4e0

Removed Google Cloud from docstrings

50e7adf

andrewsg approved these changes Mar 16, 2018

View reviewed changes

lesv approved these changes Mar 16, 2018

View reviewed changes

averikitsch merged commit 90a1166 into GoogleCloudPlatform:dlp Mar 16, 2018

averikitsch deleted the dlp-jobs branch July 16, 2018 20:12

Comments

Conversation

averikitsch commented Mar 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesv commented Mar 16, 2018

Uh oh!

lesv commented Mar 16, 2018

Uh oh!

theacodes commented Mar 16, 2018

Uh oh!

andrewsg commented Mar 17, 2018 via email

Uh oh!

theacodes commented Mar 17, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants