Check in Cloud Job Discovery client code samples for python #1546

xinyunh0929 · Jun 29, 2018

CJD got Public beta launch so we would like to have our samples checked in as other projects.

xinyunh0929 · Jun 29, 2018

Still working on the samples. But I would like to know how to run local tests for my samples.
I saw there are "_test" files under other project, but it seems I could not run it directly.

xinyunh0929 · Jul 3, 2018

I am working on the tests to those samples, however, I don't know how to run those '_test' files. Is there any guide?

googlebot · Jul 16, 2018

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

michaelawyu · Jul 16, 2018

jobs/auto_complete_sample.py

+from googleapiclient.discovery import build
+
+client_service = build('jobs', 'v2')
+


You can remove these empty lines between code and region tag.

I use pyformat -i -s 4 --recursive . to format my code. And it seems this is recommended.
Should I manually modify those files? Or it should be fine?

michaelawyu · Jul 16, 2018

jobs/auto_complete_sample.py

+
+import time
+
+# [START instantiate]


For the region tag it might be better if you add the product name (job, maybe) as a prefix, i.e. use job_setup instead of setup.

This is for tag consistence among different languages since TW prefer them in the same name. Do you think it is necessary to modify the tag?

michaelawyu · Jul 16, 2018

jobs/base_company_sample.py

+# [END instantiate]
+
+
+# [START basic_company]


Maybe it's not necessary to declare a function here with a specific region tag. How about just specifying a company variable?

company = { 'display_name': 'Google', 'distributor_company_id': distributor_company_id, 'hq_location': '1600 Amphitheatre Parkway Mountain View, CA 94043' }

You can use a fixed ID here as well I think.

I could not use a fixed ID since creating an entity with same ID would lead to the failure of tests

No. This function is required from doc site and I couldn't use the fixed ID due to our system.

michaelawyu · Jul 16, 2018

jobs/auto_complete_sample.py

+        complete.companyName = company_name
+
+    results = complete.execute()
+    print('==========\n%s\n==========' % results)


The padding part ========== may not be necessary.

Just want to make the output clear for customer. I could remove it if you think it is the better.

michaelawyu · Jul 16, 2018

jobs/base_company_sample.py

+            body=company_to_be_created).execute()
+        print('==========\nCompany created: %s\n==========' % company_created)
+        return company_created
+    except HttpError as e:


Use the base error class (googleapiclient.errors.Error) would be better I think. Or you could just drop the try/except block and the let the error raise.

michaelawyu · Jul 16, 2018

jobs/base_job_sample.py

+
+
+# [START basic_job]
+def generate_job_with_required_fields(company_name):


I feel that this can be a simple job variable as well.

Just to distinguish this function from other generate job function such as generate_job_with_custom_attribute

michaelawyu · Jul 16, 2018

jobs/batch_operation_sample.py

+
+# [START batch_job_create]
+def batch_job_create(client_service, company_name):
+    import base_job_sample


I understand your concern here; but for clarity I do feel that this part can be replaced with two simple job variables so that developers do not need to read another file to understand what's going on behind the scenes.

It could be changed later since it request modifying the samples in all languages.

michaelawyu · Jul 17, 2018

jobs/batch_operation_sample.py

+        job_to_be_updated = jobs_to_be_updated[index]
+        job_to_be_updated.update({'job_title': 'Engineer in Mountain View'})
+        request = {'job': job_to_be_updated}
+        if index % 2 == 0:


This part might be a little bit confusing I think. If we are showing the batch updating method do we really need to showcase updating only half of the jobs and leaving the other half alone? We can just update all of them.

They want to have the updateRequest one with the field mask feature, one is not.

michaelawyu · Jul 17, 2018

jobs/batch_operation_sample.py

+            pass
+
+    batch = client_service.new_batch_http_request()
+    for index in range(0, len(jobs_to_be_updated)):


Side note: if you switch to updating all jobs instead, this part can become for job in jobs, which is easier to understand and more idiomatic for Python devs.

I know. However, as mentioned above, they want half jobs updated using field mask feature, and half not.

michaelawyu · Jul 17, 2018

jobs/batch_operation_sample.py

+            pass
+
+    batch = client_service.new_batch_http_request()
+    for index in range(0, len(jobs_to_be_deleted)):


The index is not necessary though; how about for job in jobs?

michaelawyu · Jul 17, 2018

jobs/env/bin/activate.csh

@@ -0,0 +1,36 @@
+# This file must be used with "source bin/activate.csh" *from csh*.
