-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add Jobs samples #1405
add Jobs samples #1405
Conversation
dlp/jobs.py
Outdated
type: (Optional) The type of job. Defaults to 'INSPECT'. | ||
Choices: | ||
DLP_JOB_TYPE_UNSPECIFIED | ||
INSPECT_JOB: The job inspected Google Cloud for sensitive data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps change this to "The job inspected content for sensitive data"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any further advice to give on what valid filters look like? It's okay if the answer is "no".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
parent = dlp.project_path(project) | ||
|
||
# If job type is specified, convert job type to number through enums. | ||
from google.cloud.dlp_v2 import enums |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do without this import and just replace enums with google.cloud.dlp.enums where they appear below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
# 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 = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
# Iterate over results. | ||
for job in response: | ||
print('Job: %s; status: %s' % (job.name, job.JobState.Name(job.state))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the .format() method of string replacement for consistency with other samples in this directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline before Returns:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
dlp/jobs.py
Outdated
'filter.') | ||
list_parser.add_argument( | ||
'project', | ||
help='The Google Cloud project id to use as a parent resource.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider striking The Google Cloud
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dlp/jobs.py
Outdated
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.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
dlp/jobs_test.py
Outdated
import pytest | ||
import os | ||
|
||
GCLOUD_PROJECT = os.getenv('GCLOUD_PROJECT') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we prefer GOOGLE_CLOUD_PROJECT for both sides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonparrott PTAL
@broady PTAL
We were supposed to standardized on GOOGLE_CLOUD_PROJECT over two years ago.
@averikitsch Can you merge, or do you need Andrew or I to do it? |
@andrewsg @jonparrott This repo should really have kokoro or something checking on submit. |
It has travis. No idea why it's not running. |
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>
.
|
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>
.
|
No description provided.