run hooks in correct order, fixes #590 #601
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I don't totally understand this fix. Wouldn't these two have the same ordering in the output? |
drewbanin
commented
Nov 20, 2017
| compiled_hooks.append(hook_dict) | ||
|
|
||
| for hook in compiled_hooks: | ||
| ordered_hooks = sorted(compiled_hooks, key=lambda h: h.get('index', 0)) |
There was a problem hiding this comment.
@cmcarthur the actual fix is that we sort by this index here, and run the hooks in sorted order
There was a problem hiding this comment.
i get that, but when would the index ordering here be different than the ordering of hooks
|
Let's run these tests on py2 and confirm that it fails. Consider also writing unit tests |
drewbanin
pushed a commit
that referenced
this pull request
Feb 27, 2018
* only load hooks and archives once (#540) * sets schema for node before parsing raw sql (#541) * Fix/env vars (#543) * fix for bad env_var exception * overwrite target with compiled values * fixes env vars, adds test. Auto-compile profile/target args * improvements for code that runs in hooks (#544) * improvements for code that runs in hooks * fix error message note * typo * Update CHANGELOG.md * bump version (#546) * add scope to service account json creds initializer (#547) * bump 0.9.0a3 --> 0.9.0a4 (#548) * Fix README links (#554) * Update README.md * handle empty profiles.yml file (#555) * return empty string (instead of None) to avoid polluting rendered sql (#566) * tojson was added in jinja 2.9 (#563) * tojson was added in jinja 2.9 * requirements * fix package-defined schema test macros (#562) * fix package-defined schema test macros * create a dummy Relation in parsing * fix for bq quoting (#565) * bump snowflake, remove pyasn1 (#570) * bump snowflake, remove pyasn1 * change requirements.txt * allow macros to return non-text values (#571) * revert jinja version, implement tojson hack (#572) * bump to 090a5 * update changelog * bump (#574) * 090 docs (#575) * 090 docs * Update CHANGELOG.md * Update CHANGELOG.md * Raise CompilationException on duplicate model (#568) * Raise CompilationException on duplicate model Extend tests * Ignore disabled models in parse_sql_nodes Extend tests for duplicate model * Fix preexisting models * Use double quotes consistently Rename model-1 to model-disabled * Fix unit tests * Raise exception on duplicate model across packages Extend tests * Make run_started_at timezone aware (#553) (#556) * Make run_started_at timezone aware Set run_started_at timezone to UTC Enable timezone change in models Extend requirements Extend tests * Address comments from code review Create modules namespace to context Move pytz to modules Add new dependencies to setup.py * Add warning for missing constraints. Fixes #592 (#600) * Add warning for missing constraints. Fixes #592 * fix unit tests * fix schema tests used in, or defined in packages (#599) * fix schema tests used in, or defined in packages * don't hardcode dbt test namespace * fix/actually run tests * rm junk * run hooks in correct order, fixes #590 (#601) * run hooks in correct order, fixes #590 * add tests * fix tests * pep8 * change req for snowflake to fix crypto install issue (#612) From cffi callback <function _verify_callback at 0x06BF2978>: Traceback (most recent call last): File "c:\projects\dbt\.tox\pywin\lib\site-packages\OpenSSL\SSL.py", line 313, in wrapper _lib.X509_up_ref(x509) AttributeError: module 'lib' has no attribute 'X509_up_ref' From cffi callback <function _verify_callback at 0x06B8CF60>: * Update python version in Makefile from 3.5 to 3.6 (#613) * Fix/snowflake custom schema (#626) * Fixes already opened transaction issue For #602 * Fixes #621 * Create schema in archival flow (#625) * Fix for pre-hooks outside of transactions (#623) * Fix for pre-hooks outside of transactions #576 * improve tests * Fixes already opened transaction issue (#622) For #602 * Accept string for postgres port number (#583) (#624) * Accept string for postgres port number (#583) * s/str/basestring/g * print correct run time (include hooks) (#607) * add support for late binding views (Redshift) (#614) * add support for late binding views (Redshift) * fix bind logic * wip for get_columns_in_table * fix get_columns_in_table * fix for default value in bind config * pep8 * skip tests that depend on nonexistent or disabled models (#617) * skip tests that depend on nonexistent or disabled models * pep8, Fixes #616 * refactor * fix for adapter macro called within packages (#630) * fix for adapter macro called within packages * better error message * Update CHANGELOG.md (#632) * Update CHANGELOG.md * Update CHANGELOG.md * Bump version: 0.9.0 → 0.9.1 * more helpful exception for registry funcs * Rework deps to support local & git * pylint and cleanup * make modules directory first * Refactor registry client for cleanliness and better error handling * init converter script * create modules directory only if non-existent * Only check the hub registry for registry packages * Incorporate changes from Drew's branch Diff of original changes: https://github.com/fishtown-analytics/dbt/pull/591/files * lint * include a portion of the actual name in destination directory * Install dependencies using actual name; better exceptions * Error if two dependencies have same name * Process dependencies one level at a time Included in this change is a refactor of the deps run function for clarity. Also I changed the resolve_version function to update the object in place. I prefer the immutability of this function as it was, but the rest of the code doesn't really operate that way. And I ran into some bugs due to this discrepancy. * update var name * Provide support for repositories in project yml * Download files in a temp directory The downloads directory causes problems with the run command because this directory is not a dbt project. Need to download it elsewhere. * pin some versions * pep8-ify * some PR feedback changes around logging * PR feedback round 2 * Fix for redshift varchar bug (#647) * Fix for redshift varchar bug * pep8 on a sql string, smh * Set global variable overrides on the command line with --vars (#640) * Set global variable overrides on the command line with --vars * pep8 * integration tests for cli vars * Seed rewrite (#618) * loader for seed data files * Functioning rework of seed task * Make CompilerRunner fns private and impl. SeedRunner.compile Trying to distinguish between the public/private interface for this class. And the SeedRunner doesn't need the functionality in the compile function, it just needs a compile function to exist for use in the compilation process. * Test changes and fixes * make the DB setup script usable locally * convert simple copy test to use seeed * Fixes to get Snowflake working * New seed flag and make it non-destructive by default * Convert update SQL script to another seed * cleanup * implement bigquery csv load * context handling of StringIO * Better typing * strip seeder and csvkit dependency * update bigquery to use new data typing and to fix unicode issue * update seed test * fix abstract functions in base adapter * support time type * try pinning crypto, pyopenssl versions * remove unnecessary version pins * insert all at once, rather than one query per row * do not quote field names on creation * bad * quiet down parsedatetime logger * pep8 * UI updates + node conformity for seed nodes * add seed to list of resource types, cleanup * show option for CSVs * typo * pep8 * move agate import to avoid strange warnings * deprecation warning for --drop-existing * quote column names in seed files * revert quoting change (breaks Snowflake). Hush warnings * use hub url * Show installed version, silence semver regex warnings * sort versions to make tests deterministic. Prefer higher versions * pep8, fix comparison functions for py3 * make compare function return value in {-1, 0, 1} * fix for deleting git dirs on windows? * use system client rmdir instead of shutil directly * debug logging to identify appveyor issue * less restrictive error retry * rm debug logging
drewbanin
pushed a commit
that referenced
this pull request
Feb 27, 2018
* semver resolution * cleanup * remove unnecessary comment * add test for multiples on both sides * add resolve_to_specific_version * local registry * hacking out deps * Buck pkg mgmt (#645) * only load hooks and archives once (#540) * sets schema for node before parsing raw sql (#541) * Fix/env vars (#543) * fix for bad env_var exception * overwrite target with compiled values * fixes env vars, adds test. Auto-compile profile/target args * improvements for code that runs in hooks (#544) * improvements for code that runs in hooks * fix error message note * typo * Update CHANGELOG.md * bump version (#546) * add scope to service account json creds initializer (#547) * bump 0.9.0a3 --> 0.9.0a4 (#548) * Fix README links (#554) * Update README.md * handle empty profiles.yml file (#555) * return empty string (instead of None) to avoid polluting rendered sql (#566) * tojson was added in jinja 2.9 (#563) * tojson was added in jinja 2.9 * requirements * fix package-defined schema test macros (#562) * fix package-defined schema test macros * create a dummy Relation in parsing * fix for bq quoting (#565) * bump snowflake, remove pyasn1 (#570) * bump snowflake, remove pyasn1 * change requirements.txt * allow macros to return non-text values (#571) * revert jinja version, implement tojson hack (#572) * bump to 090a5 * update changelog * bump (#574) * 090 docs (#575) * 090 docs * Update CHANGELOG.md * Update CHANGELOG.md * Raise CompilationException on duplicate model (#568) * Raise CompilationException on duplicate model Extend tests * Ignore disabled models in parse_sql_nodes Extend tests for duplicate model * Fix preexisting models * Use double quotes consistently Rename model-1 to model-disabled * Fix unit tests * Raise exception on duplicate model across packages Extend tests * Make run_started_at timezone aware (#553) (#556) * Make run_started_at timezone aware Set run_started_at timezone to UTC Enable timezone change in models Extend requirements Extend tests * Address comments from code review Create modules namespace to context Move pytz to modules Add new dependencies to setup.py * Add warning for missing constraints. Fixes #592 (#600) * Add warning for missing constraints. Fixes #592 * fix unit tests * fix schema tests used in, or defined in packages (#599) * fix schema tests used in, or defined in packages * don't hardcode dbt test namespace * fix/actually run tests * rm junk * run hooks in correct order, fixes #590 (#601) * run hooks in correct order, fixes #590 * add tests * fix tests * pep8 * change req for snowflake to fix crypto install issue (#612) From cffi callback <function _verify_callback at 0x06BF2978>: Traceback (most recent call last): File "c:\projects\dbt\.tox\pywin\lib\site-packages\OpenSSL\SSL.py", line 313, in wrapper _lib.X509_up_ref(x509) AttributeError: module 'lib' has no attribute 'X509_up_ref' From cffi callback <function _verify_callback at 0x06B8CF60>: * Update python version in Makefile from 3.5 to 3.6 (#613) * Fix/snowflake custom schema (#626) * Fixes already opened transaction issue For #602 * Fixes #621 * Create schema in archival flow (#625) * Fix for pre-hooks outside of transactions (#623) * Fix for pre-hooks outside of transactions #576 * improve tests * Fixes already opened transaction issue (#622) For #602 * Accept string for postgres port number (#583) (#624) * Accept string for postgres port number (#583) * s/str/basestring/g * print correct run time (include hooks) (#607) * add support for late binding views (Redshift) (#614) * add support for late binding views (Redshift) * fix bind logic * wip for get_columns_in_table * fix get_columns_in_table * fix for default value in bind config * pep8 * skip tests that depend on nonexistent or disabled models (#617) * skip tests that depend on nonexistent or disabled models * pep8, Fixes #616 * refactor * fix for adapter macro called within packages (#630) * fix for adapter macro called within packages * better error message * Update CHANGELOG.md (#632) * Update CHANGELOG.md * Update CHANGELOG.md * Bump version: 0.9.0 → 0.9.1 * more helpful exception for registry funcs * Rework deps to support local & git * pylint and cleanup * make modules directory first * Refactor registry client for cleanliness and better error handling * init converter script * create modules directory only if non-existent * Only check the hub registry for registry packages * Incorporate changes from Drew's branch Diff of original changes: https://github.com/fishtown-analytics/dbt/pull/591/files * lint * include a portion of the actual name in destination directory * Install dependencies using actual name; better exceptions * Error if two dependencies have same name * Process dependencies one level at a time Included in this change is a refactor of the deps run function for clarity. Also I changed the resolve_version function to update the object in place. I prefer the immutability of this function as it was, but the rest of the code doesn't really operate that way. And I ran into some bugs due to this discrepancy. * update var name * Provide support for repositories in project yml * Download files in a temp directory The downloads directory causes problems with the run command because this directory is not a dbt project. Need to download it elsewhere. * pin some versions * pep8-ify * some PR feedback changes around logging * PR feedback round 2 * Fix for redshift varchar bug (#647) * Fix for redshift varchar bug * pep8 on a sql string, smh * Set global variable overrides on the command line with --vars (#640) * Set global variable overrides on the command line with --vars * pep8 * integration tests for cli vars * Seed rewrite (#618) * loader for seed data files * Functioning rework of seed task * Make CompilerRunner fns private and impl. SeedRunner.compile Trying to distinguish between the public/private interface for this class. And the SeedRunner doesn't need the functionality in the compile function, it just needs a compile function to exist for use in the compilation process. * Test changes and fixes * make the DB setup script usable locally * convert simple copy test to use seeed * Fixes to get Snowflake working * New seed flag and make it non-destructive by default * Convert update SQL script to another seed * cleanup * implement bigquery csv load * context handling of StringIO * Better typing * strip seeder and csvkit dependency * update bigquery to use new data typing and to fix unicode issue * update seed test * fix abstract functions in base adapter * support time type * try pinning crypto, pyopenssl versions * remove unnecessary version pins * insert all at once, rather than one query per row * do not quote field names on creation * bad * quiet down parsedatetime logger * pep8 * UI updates + node conformity for seed nodes * add seed to list of resource types, cleanup * show option for CSVs * typo * pep8 * move agate import to avoid strange warnings * deprecation warning for --drop-existing * quote column names in seed files * revert quoting change (breaks Snowflake). Hush warnings * use hub url * Show installed version, silence semver regex warnings * sort versions to make tests deterministic. Prefer higher versions * pep8, fix comparison functions for py3 * make compare function return value in {-1, 0, 1} * fix for deleting git dirs on windows? * use system client rmdir instead of shutil directly * debug logging to identify appveyor issue * less restrictive error retry * rm debug logging * s/version/revision for git packages * more s/version/revision, deprecation cleanup * remove unused semver codepath * plus symlinks!!! * get rid of reference to removed function
iknox-fa
pushed a commit
that referenced
this pull request
Feb 8, 2022
* semver resolution * cleanup * remove unnecessary comment * add test for multiples on both sides * add resolve_to_specific_version * local registry * hacking out deps * Buck pkg mgmt (#645) * only load hooks and archives once (#540) * sets schema for node before parsing raw sql (#541) * Fix/env vars (#543) * fix for bad env_var exception * overwrite target with compiled values * fixes env vars, adds test. Auto-compile profile/target args * improvements for code that runs in hooks (#544) * improvements for code that runs in hooks * fix error message note * typo * Update CHANGELOG.md * bump version (#546) * add scope to service account json creds initializer (#547) * bump 0.9.0a3 --> 0.9.0a4 (#548) * Fix README links (#554) * Update README.md * handle empty profiles.yml file (#555) * return empty string (instead of None) to avoid polluting rendered sql (#566) * tojson was added in jinja 2.9 (#563) * tojson was added in jinja 2.9 * requirements * fix package-defined schema test macros (#562) * fix package-defined schema test macros * create a dummy Relation in parsing * fix for bq quoting (#565) * bump snowflake, remove pyasn1 (#570) * bump snowflake, remove pyasn1 * change requirements.txt * allow macros to return non-text values (#571) * revert jinja version, implement tojson hack (#572) * bump to 090a5 * update changelog * bump (#574) * 090 docs (#575) * 090 docs * Update CHANGELOG.md * Update CHANGELOG.md * Raise CompilationException on duplicate model (#568) * Raise CompilationException on duplicate model Extend tests * Ignore disabled models in parse_sql_nodes Extend tests for duplicate model * Fix preexisting models * Use double quotes consistently Rename model-1 to model-disabled * Fix unit tests * Raise exception on duplicate model across packages Extend tests * Make run_started_at timezone aware (#553) (#556) * Make run_started_at timezone aware Set run_started_at timezone to UTC Enable timezone change in models Extend requirements Extend tests * Address comments from code review Create modules namespace to context Move pytz to modules Add new dependencies to setup.py * Add warning for missing constraints. Fixes #592 (#600) * Add warning for missing constraints. Fixes #592 * fix unit tests * fix schema tests used in, or defined in packages (#599) * fix schema tests used in, or defined in packages * don't hardcode dbt test namespace * fix/actually run tests * rm junk * run hooks in correct order, fixes #590 (#601) * run hooks in correct order, fixes #590 * add tests * fix tests * pep8 * change req for snowflake to fix crypto install issue (#612) From cffi callback <function _verify_callback at 0x06BF2978>: Traceback (most recent call last): File "c:\projects\dbt\.tox\pywin\lib\site-packages\OpenSSL\SSL.py", line 313, in wrapper _lib.X509_up_ref(x509) AttributeError: module 'lib' has no attribute 'X509_up_ref' From cffi callback <function _verify_callback at 0x06B8CF60>: * Update python version in Makefile from 3.5 to 3.6 (#613) * Fix/snowflake custom schema (#626) * Fixes already opened transaction issue For #602 * Fixes #621 * Create schema in archival flow (#625) * Fix for pre-hooks outside of transactions (#623) * Fix for pre-hooks outside of transactions #576 * improve tests * Fixes already opened transaction issue (#622) For #602 * Accept string for postgres port number (#583) (#624) * Accept string for postgres port number (#583) * s/str/basestring/g * print correct run time (include hooks) (#607) * add support for late binding views (Redshift) (#614) * add support for late binding views (Redshift) * fix bind logic * wip for get_columns_in_table * fix get_columns_in_table * fix for default value in bind config * pep8 * skip tests that depend on nonexistent or disabled models (#617) * skip tests that depend on nonexistent or disabled models * pep8, Fixes #616 * refactor * fix for adapter macro called within packages (#630) * fix for adapter macro called within packages * better error message * Update CHANGELOG.md (#632) * Update CHANGELOG.md * Update CHANGELOG.md * Bump version: 0.9.0 → 0.9.1 * more helpful exception for registry funcs * Rework deps to support local & git * pylint and cleanup * make modules directory first * Refactor registry client for cleanliness and better error handling * init converter script * create modules directory only if non-existent * Only check the hub registry for registry packages * Incorporate changes from Drew's branch Diff of original changes: https://github.com/fishtown-analytics/dbt/pull/591/files * lint * include a portion of the actual name in destination directory * Install dependencies using actual name; better exceptions * Error if two dependencies have same name * Process dependencies one level at a time Included in this change is a refactor of the deps run function for clarity. Also I changed the resolve_version function to update the object in place. I prefer the immutability of this function as it was, but the rest of the code doesn't really operate that way. And I ran into some bugs due to this discrepancy. * update var name * Provide support for repositories in project yml * Download files in a temp directory The downloads directory causes problems with the run command because this directory is not a dbt project. Need to download it elsewhere. * pin some versions * pep8-ify * some PR feedback changes around logging * PR feedback round 2 * Fix for redshift varchar bug (#647) * Fix for redshift varchar bug * pep8 on a sql string, smh * Set global variable overrides on the command line with --vars (#640) * Set global variable overrides on the command line with --vars * pep8 * integration tests for cli vars * Seed rewrite (#618) * loader for seed data files * Functioning rework of seed task * Make CompilerRunner fns private and impl. SeedRunner.compile Trying to distinguish between the public/private interface for this class. And the SeedRunner doesn't need the functionality in the compile function, it just needs a compile function to exist for use in the compilation process. * Test changes and fixes * make the DB setup script usable locally * convert simple copy test to use seeed * Fixes to get Snowflake working * New seed flag and make it non-destructive by default * Convert update SQL script to another seed * cleanup * implement bigquery csv load * context handling of StringIO * Better typing * strip seeder and csvkit dependency * update bigquery to use new data typing and to fix unicode issue * update seed test * fix abstract functions in base adapter * support time type * try pinning crypto, pyopenssl versions * remove unnecessary version pins * insert all at once, rather than one query per row * do not quote field names on creation * bad * quiet down parsedatetime logger * pep8 * UI updates + node conformity for seed nodes * add seed to list of resource types, cleanup * show option for CSVs * typo * pep8 * move agate import to avoid strange warnings * deprecation warning for --drop-existing * quote column names in seed files * revert quoting change (breaks Snowflake). Hush warnings * use hub url * Show installed version, silence semver regex warnings * sort versions to make tests deterministic. Prefer higher versions * pep8, fix comparison functions for py3 * make compare function return value in {-1, 0, 1} * fix for deleting git dirs on windows? * use system client rmdir instead of shutil directly * debug logging to identify appveyor issue * less restrictive error retry * rm debug logging * s/version/revision for git packages * more s/version/revision, deprecation cleanup * remove unused semver codepath * plus symlinks!!! * get rid of reference to removed function automatic commit by git-black, original commits: 5fbcd12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
in 0.9.0,
on-run-startandon-run-endhooks are stored in a dictionary between parsing and running. This means that the order of hook execution is not guaranteed to be the order in which the hooks are defined.This PR adds an
indexto hook dicts. The dictionary of hooks is sorted by this index before running. This ensures that hooks run in the same order that they're defined.