Skip to content
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

Enable environment overrides for job clusters #658

Merged
merged 2 commits into from
Aug 14, 2023

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Aug 11, 2023

Changes

While they are a slice, we can identify a job cluster by its job cluster key. A job definition with multiple job clusters with the same key is always invalid. We can therefore merge definitions with the same key into one. This is compatible with how environment overrides are applied; merging a slice means appending to it. The override will end up in the job cluster slice of the original, which gives us a deterministic way to merge them.

Since the alternative is an invalid configuration, this doesn't change behavior.

Tests

New test coverage.

While they are a slice, we can identify a job cluster by its job cluster key.
A job definition with multiple job clusters with the same key is always
invalid.  We can therefore merge definitions with the same key into one.
This is compatible with how environment overrides are applied; merging a slice
means appending to it. The override will end up in the job cluster slice of the
original, which gives us a deterministic way to merge them.
@andrewnester
Copy link
Contributor

LGTM! To clarify: if I don’t specify job cluster in resources section but only in environment overrides, bundle would still work, correct?

@pietern
Copy link
Contributor Author

pietern commented Aug 14, 2023

@andrewnester That is correct; the sections are appended before doing this type of processing.

@pietern pietern added this pull request to the merge queue Aug 14, 2023
Merged via the queue into main with commit 97699b8 Aug 14, 2023
4 checks passed
@pietern pietern deleted the job-clusters-merge-same-key branch August 14, 2023 06:52
pietern added a commit that referenced this pull request Aug 16, 2023
CLI:
 * Always resolve .databrickscfg file ([#659](#659)).

Bundles:
 * Add internal tag for bundle fields to be skipped from schema ([#636](#636)).
 * Log the bundle root configuration file if applicable ([#657](#657)).
 * Execute paths without the .tmpl extension as templates ([#654](#654)).
 * Enable environment overrides for job clusters ([#658](#658)).
 * Merge artifacts and resources block with overrides enabled ([#660](#660)).
 * Locked terraform binary version to <= 1.5.5 ([#666](#666)).
 * Return better error messages for invalid JSON schema types in templates ([#661](#661)).
 * Use custom prompter for bundle template inputs ([#663](#663)).
 * Add map and pair helper functions for bundle templates ([#665](#665)).
 * Correct name for force acquire deploy flag ([#656](#656)).
 * Confirm that override with a zero value doesn't work ([#669](#669)).

Internal:
 * Consolidate functions in libs/git ([#652](#652)).
 * Upgraded Go version to 1.21 ([#664](#664)).
@pietern pietern mentioned this pull request Aug 16, 2023
github-merge-queue bot pushed a commit that referenced this pull request Aug 16, 2023
CLI:
* Always resolve .databrickscfg file
([#659](#659)).

Bundles:
* Add internal tag for bundle fields to be skipped from schema
([#636](#636)).
* Log the bundle root configuration file if applicable
([#657](#657)).
* Execute paths without the .tmpl extension as templates
([#654](#654)).
* Enable environment overrides for job clusters
([#658](#658)).
* Merge artifacts and resources block with overrides enabled
([#660](#660)).
* Locked terraform binary version to <= 1.5.5
([#666](#666)).
* Return better error messages for invalid JSON schema types in
templates ([#661](#661)).
* Use custom prompter for bundle template inputs
([#663](#663)).
* Add map and pair helper functions for bundle templates
([#665](#665)).
* Correct name for force acquire deploy flag
([#656](#656)).
* Confirm that override with a zero value doesn't work
([#669](#669)).

Internal:
* Consolidate functions in libs/git
([#652](#652)).
* Upgraded Go version to 1.21
([#664](#664)).
@stawo
Copy link

stawo commented Aug 23, 2023

@pietern Do you think it would be possible to apply the same logic to the tasks of a job?
Example: I want to set specific clusters for a task (blatantly copied your example)

resources:
  jobs:
    foo:
      name: job
      tasks:
        - task_key: key
          new_cluster:
            spark_version: 13.3.x-scala2.12

environments:
  development:
    resources:
      jobs:
        foo:
          tasks:
            - task_key: key
              new_cluster:
                node_type_id: i3.xlarge
                num_workers: 1

  staging:
    resources:
      jobs:
        foo:
          tasks:
            - task_key: key
              new_cluster:
                node_type_id: i3.2xlarge
                num_workers: 4

I never coded in GO, but if you say it would be exactly the same as your changes (only applied to the task element), I could try to do it.

github-merge-queue bot pushed a commit that referenced this pull request Sep 18, 2023
## Changes
Follow up for #658

When a job definition has multiple job tasks using the same key, it's
considered invalid. Instead we should combine those definitions with the
same key into one. This is consistent with environment overrides. This
way, the override ends up in the original job tasks, and we've got a
clear way to put them all together.

## Tests
Added unit tests
@pietern
Copy link
Contributor Author

pietern commented Sep 18, 2023

@stawo The behavior you propose is implemented in the linked PR (#779).

@stawo
Copy link

stawo commented Sep 19, 2023

@pietern , yes, I saw it the other day. Amazing, looking forward to try it in the upcoming releases 💯

pietern added a commit that referenced this pull request Sep 21, 2023
This is a follow up to #658 and #779 for jobs.

This change applies label normalization the same way the backend does.
github-merge-queue bot pushed a commit that referenced this pull request Sep 21, 2023
## Changes

This is a follow-up to #658 and #779 for jobs.

This change applies label normalization the same way the backend does.

## Tests

Unit and config loading tests.
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
## Changes
Follow up for #658

When a job definition has multiple job tasks using the same key, it's
considered invalid. Instead we should combine those definitions with the
same key into one. This is consistent with environment overrides. This
way, the override ends up in the original job tasks, and we've got a
clear way to put them all together.

## Tests
Added unit tests
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
This is a follow-up to #658 and #779 for jobs.

This change applies label normalization the same way the backend does.

Unit and config loading tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants