-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix dbt production deploy problems #82
Fix dbt production deploy problems #82
Conversation
- name: Configure dbt environment | ||
uses: ./.github/actions/configure_dbt_environment |
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.
Run dbt docs generate --target "$TARGET"
18:26:[23](https://github.com/ccao-data/data-architecture/actions/runs/5905883640/job/16020814093#step:6:24) Running with dbt=1.5.5
18:[26](https://github.com/ccao-data/data-architecture/actions/runs/5905883640/job/16020814093#step:6:27):23 Encountered an error:
Runtime Error
The profile 'athena' does not have a target named ''. The valid target names for this profile are:
- dev
- ci
- prod
Error: Process completed with exit code 2.
Previously we had been hardcoding the target "ci"
so I hadn't noticed that we were missing this step.
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.
Nice, I should've caught this too. Tricky tricky
e31e62e
to
068ddad
Compare
- name: Move dbt manifest directory to update cache | ||
run: mv "$TARGET_DIR" "$STATE_DIR" | ||
working-directory: ${{ env.PROJECT_DIR }} | ||
shell: bash |
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.
Run mv "$TARGET_DIR" "$STATE_DIR"
mv: cannot move 'target' to 'state/target': Directory not empty
Error: Process completed with exit code 1.
Saving the cache explicitly with the actions/cache/save
action resolves this problem.
# We have to use the separate `restore`/`save` actions instead of the | ||
# unified `cache` action because only `restore` provides access to the | ||
# `cache-matched-key` and `cache-primary-key` outputs as of v3 |
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.
This is a frustrating difference! Note the outputs of the restore
action vs. the outputs of the cache
action.
# If we restore the cache from the `restore-keys` key, the `cache-hit` | ||
# output will be 'false' but the `cache-matched-key` output will be | ||
# the name of the `restore-keys` key; we want to count this case as a hit | ||
- if: | | ||
steps.cache.outputs.cache-hit == 'true' || | ||
steps.cache.outputs.cache-matched-key == format( | ||
'{0}-master', env.CACHE_NAME | ||
) |
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.
This workflow run has three attempts where I tested each of the possible cases:
- Attempt #1: Branch cache exists; exact cache key hit ➡️ this block evaluates True
- Attempt #2: Deleted branch cache, but
master
branch cache exists; partial cache key hit ➡️ this block evaluates True - Attempt #3: Deleted both branch cache and
master
branch cache; no cache key hit ➡️ this block evaluates False
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.
praise: Sheesh, this is very confusing behavior. And weird that the actions maintainers don't provide access to the cache-
vars in actions/cache
. Nice work on the fast debugging!
- name: Configure dbt environment | ||
uses: ./.github/actions/configure_dbt_environment |
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.
Nice, I should've caught this too. Tricky tricky
# If we restore the cache from the `restore-keys` key, the `cache-hit` | ||
# output will be 'false' but the `cache-matched-key` output will be | ||
# the name of the `restore-keys` key; we want to count this case as a hit | ||
- if: | | ||
steps.cache.outputs.cache-hit == 'true' || | ||
steps.cache.outputs.cache-matched-key == format( | ||
'{0}-master', env.CACHE_NAME | ||
) |
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.
praise: Sheesh, this is very confusing behavior. And weird that the actions maintainers don't provide access to the cache-
vars in actions/cache
. Nice work on the fast debugging!
Three problems that emerged out of merging the data catalog branch into master (#78):
mv
call that is not actually performing the correct directory renaming$MODIFIED_RESOURCES_ONLY
can be set tofalse
when we want it to betrue
in cases where we have to restore the cache from therestore-keys
fallbackThis PR fixes those three problems.