Feat/abstract configs #38

matt-winkler · 2022-09-27T01:04:05Z

No description provided.

codecov · 2022-09-27T01:08:26Z

Codecov Report

Base: 86.15% // Head: 84.61% // Decreases project coverage by -1.54% ⚠️

Coverage data is based on head (db38153) compared to base (8b56460).
Patch coverage: 71.00% of modified lines in pull request are covered.

❗ Current head db38153 differs from pull request most recent head cfea827. Consider uploading reports for the commit cfea827 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
- Coverage   86.15%   84.61%   -1.55%     
==========================================
  Files          13       15       +2     
  Lines        1127     1183      +56     
==========================================
+ Hits          971     1001      +30     
- Misses        156      182      +26

Impacted Files	Coverage Δ
dbtc/client/cloud/configs/dbt_cloud_api.py	`42.85% <42.85%> (ø)`
dbtc/client/cloud/base.py	`77.77% <74.39%> (-2.82%)`	⬇️
dbtc/cli.py	`74.52% <100.00%> (+0.09%)`	⬆️
dbtc/client/cloud/configs/dbt_core_cli.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

dbtc/client/cloud/configs/dbt_cloud_api.py

matt-winkler · 2022-09-27T15:58:04Z

@dpguthrie There's a lot in this PR, so to help organize thoughts I wanted to lay out a few areas where feedback would be valuable:

The notion of templating our API requests in dbt_cloud_api
Adding the concept of mode to trigger_job. My thinking here is to make it more clear for the end user that they only get to run a job in a single mode at a time. Otherwise I could see someone passing e.g. restart_from_failure=True AND autoscale=True and be confused about what behavior to expect.
As the number of use cases grows, the trigger_job method is going to have lots of possible parameters. Are we OK with that or would it be valuable to expose the concept of run modes in separate methods callable from the CLI, e.g. dbtc trigger-job-with-restart-from-failure, dbtc trigger-job-with-autoscale, etc.

dpguthrie · 2022-09-27T22:17:21Z

The notion of templating our API requests in dbt_cloud_api

I came up with a different approach for validation using the pydantic library. This would both do validation of the payload that a user specifies as well as give it the appropriate defaults (id for instance).

#39

We can iterate on what's best, I just didn't want to overwrite all your work.

dpguthrie · 2022-09-27T22:46:48Z

As the number of use cases grows, the trigger_job method is going to have lots of possible parameters. Are we OK with that or would it be valuable to expose the concept of run modes in separate methods callable from the CLI, e.g. dbtc trigger-job-with-restart-from-failure, dbtc trigger-job-with-autoscale, etc.

Putting this here as well - my take is that we should be building out different methods that accomplish these different tasks. The trigger_job method is already probably a bit too big and doing too much. It should be the entry point to trigger a job from all those other methods, but each of those methods will have logic that doesn't make sense to wrap all together. Also, I'm guessing you're just gonna continue thinking of awesome use cases for this library, so we should plan for that.

matt-winkler · 2022-09-27T23:12:42Z

@dpguthrie 100 on breaking out separate methods that all get to the base trigger_job. I was mid-way on that before thinking to stop and ask you :))

matt-winkler · 2022-09-28T18:25:47Z

dbtc/client/cloud/base.py


        return 'api_key'

+    def _clone_resource(self, resource: str, **kwargs):


@dpguthrie this is very cool! Is there a means I could use to subset which fields from the base object are used for replication? It seems like this is the intent of create_args, but I think a field subset being passed there would need to affect the payload in order for that modification to occur.

Context: When trying to replicate a job, I'm not able to use fields such as deferring_job_definition_id on the CREATE JOB endpoint. The process needs to first create the job with fewer fields, and then add the deferring_job_defintion_id in an update.

Ya, we need another argument here for that. Good call! Who knows this private method may be a bit overkill for the things we need to do as well as thinking about what resources actually need to be cloned.

Per conversation offline, noting that we agreed this isn't something to pursue further for now. There will be too much situational variation in what's needed to replicate resources, so we should stick to the CRUD operations and adjust as-needed for other use cases.

dpguthrie · 2022-09-29T18:05:47Z

dbtc/client/cloud/base.py

+            new_job_definition = {k:v for k,v in existing_job_defintion.items() if k in required_fields}
+
+            if not autoscale_job_identifier:
+                creation_time = datetime.now().strftime('%Y-%m-%d-%H-%M-%S')
+                new_job_name = '-'.join([new_job_definition['name'], creation_time])
+                new_job_definition['name'] = new_job_name
+            else:
+                new_job_name = '-'.join([new_job_definition['name'], autoscale_job_identifier])
+                new_job_definition['name'] = new_job_name
+
+            # make sure the id is none on the new job
+            new_job_definition['id'] = None
+            job_id = self.create_job(
+                account_id=account_id, payload=new_job_definition
+            )['data']['id']
+
+            # fill in optional fields on the job if present in the source
+            for field in optional_fields:
+                new_job_definition[field] = existing_job_defintion.get(field)
+
+            # set the ID in the update request
+            new_job_definition['id'] = job_id
+            self.console.log(new_job_definition)
+            job_id = self.update_job(
+                account_id=account_id, job_id=job_id, payload=new_job_definition
+            )['data']['id']


@matt-winkler I'm not sure I follow here. Why wouldn't we just create the new job from the existing job definition (outside of setting id to None and changing the name)?

@dpguthrie we can't copy the existing data in the job 1:1 in a new CREATE JOB request. For example, you can't create a new job including the is_deferrable parameter. That parameter needs to be applied in a subsequent UPDATE step.

@dpguthrie updated my comment with a better example

matt-winkler

🔥

…n trigger_job method

…o feat/abstract-configs

matt-winkler · 2022-10-08T14:25:51Z

@dpguthrie Think I found a happier medium with the cloning idea. This updates _clone_resource so that it will return a payload that includes the object's required fields + and optional fields that are not None in the source data. Then we can further modify depending on the use case e.g. 1) in autoscaling we want to add an identifier to the job's name OR 2) if this was for migration, just push the same payload to another account / project etc.

matt-winkler · 2022-10-08T14:36:53Z

Also adding a comment that tests on autoscaling jobs are failing because we need to standardize on an account / jobs to work from.

matt-winkler added 6 commits September 15, 2022 17:27

draft version

6a8a156

Merge branch 'main' into feat/trigger-ci-job-in-parallel

42d2de9

demo-able version

680ffc0

create configs directory

15bf9e1

sketch configs idea

917b4b2

adds autoscaling mode and moves some configs out of base _CloudClient

db38153

dpguthrie reviewed Sep 27, 2022

View reviewed changes

dbtc/client/cloud/configs/dbt_cloud_api.py Outdated Show resolved Hide resolved

matt-winkler and others added 6 commits September 27, 2022 10:52

create enums file

1eb1683

update enums

d083cc2

add autoscale name slug

593e000

rename mode --> job_run_strategy

191f199

Add pydantic

59bbc9f

Add support for cloning, resource validation

ccea05c

matt-winkler added 3 commits September 28, 2022 09:07

fix merge conflicts

70880e1

fix merge conflicts

299f391

merge feat/model-validation changes

acf0b8e

matt-winkler commented Sep 28, 2022

View reviewed changes

matt-winkler added 2 commits September 28, 2022 14:36

autoscaling works again

c718176

add test for autoscaling

0ba0df1

dpguthrie reviewed Sep 29, 2022

View reviewed changes

Update job

e22551d

matt-winkler commented Sep 29, 2022

View reviewed changes

dpguthrie and others added 3 commits September 30, 2022 15:25

Add option for multiple status in list_runs method

767582d

Add tests for new status functionality

4922990

update restart_job_from_failure to trigger_job_restart_from_failure i…

0ce7118

…n trigger_job method

matt-winkler added 3 commits October 7, 2022 14:44

Merge branch 'feat/abstract-configs' of github.com:dpguthrie/dbtc int…

cfea827

…o feat/abstract-configs

update required fields for Job to include dbt_version

ce98cde

_clone_resource gives a base payload to be modified by use case

2a137a7

matt-winkler closed this Jul 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feat/abstract configs #38

Feat/abstract configs #38

Uh oh!

matt-winkler commented Sep 27, 2022

Uh oh!

codecov bot commented Sep 27, 2022 •

edited

Loading

Uh oh!

Uh oh!

matt-winkler commented Sep 27, 2022 •

edited

Loading

Uh oh!

dpguthrie commented Sep 27, 2022 •

edited

Loading

Uh oh!

dpguthrie commented Sep 27, 2022

Uh oh!

matt-winkler commented Sep 27, 2022

Uh oh!

matt-winkler Sep 28, 2022

Uh oh!

dpguthrie Sep 28, 2022

Uh oh!

matt-winkler Sep 29, 2022

Uh oh!

dpguthrie Sep 29, 2022

Uh oh!

matt-winkler Sep 29, 2022 •

edited

Loading

Uh oh!

matt-winkler Sep 29, 2022

Uh oh!

matt-winkler left a comment

Uh oh!

matt-winkler commented Oct 8, 2022

Uh oh!

matt-winkler commented Oct 8, 2022

Uh oh!

Uh oh!


		return 'api_key'

		def _clone_resource(self, resource: str, **kwargs):

Feat/abstract configs #38

Feat/abstract configs #38

Uh oh!

Conversation

matt-winkler commented Sep 27, 2022

Uh oh!

codecov bot commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

matt-winkler commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dpguthrie commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dpguthrie commented Sep 27, 2022

Uh oh!

matt-winkler commented Sep 27, 2022

Uh oh!

matt-winkler Sep 28, 2022

Choose a reason for hiding this comment

Uh oh!

dpguthrie Sep 28, 2022

Choose a reason for hiding this comment

Uh oh!

matt-winkler Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

dpguthrie Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

matt-winkler Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matt-winkler Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

matt-winkler left a comment

Choose a reason for hiding this comment

Uh oh!

matt-winkler commented Oct 8, 2022

Uh oh!

matt-winkler commented Oct 8, 2022

Uh oh!

Uh oh!

codecov bot commented Sep 27, 2022 •

edited

Loading

matt-winkler commented Sep 27, 2022 •

edited

Loading

dpguthrie commented Sep 27, 2022 •

edited

Loading

matt-winkler Sep 29, 2022 •

edited

Loading