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

[Backport] #80 to unstick releases for adapters #86

Closed
wants to merge 2 commits into from

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented Mar 21, 2022

resolves #80 second half in which we need to control what branches our pr's tests against

Description

Pulls in #80 to latest branch to clear up tests against wrong branches.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-redshift next" section.

@McKnight-42 McKnight-42 self-assigned this Mar 21, 2022
@cla-bot cla-bot bot added the cla:yes label Mar 21, 2022
@McKnight-42
Copy link
Contributor Author

Was curious if as this is the newest of the things that will be backported if removing the mentions of previous changes that will be added to back port ex. #52 and #63.

@leahwicz
Copy link
Contributor

It looks like we are still failing tests with this updated workflow. Not sure what is wrong but let's dig into it and see if we really have a bug in the 1.0.latest branches or if its the CI

@McKnight-42
Copy link
Contributor Author

McKnight-42 commented Mar 23, 2022

@leahwicz most of the failures for unit test checks are due to print not referenced or removed from the loop when comparing the required named variables to the potential variables and the references from the class due to BaseContext class in dbt-core having a method called print that gets wrapped up in the ctx variable

def assert_has_keys(
    required_keys: Set[str], maybe_keys: Set[str], ctx: Dict[str, Any]
):
    keys = set(ctx)
    for key in required_keys:
        assert key in keys, f'{key} in required keys but not in context'
        keys.remove(key)
    extras = keys.difference(maybe_keys)
    assert not extras, f'got extra keys in context: {extras}'

example of breakpoint breakdown of variables

 print(keys) - set of keys from ctx variable (BaseContext)
{'toyaml', 'fromyaml', 'log', 'fromjson', 'context', 'modules', 'tojson', 'dbt_version', 'return', 'flags', 'builtins', 'env_var', 'var', 'invocation_id', 'print', 'run_started_at'}
 print(required_keys) - Required defined keys (defined by hardcode on test_context.py)
frozenset({'return', 'toyaml', 'fromyaml', 'log', 'fromjson', 'flags', 'builtins', 'env_var', 'context', 'modules', 'var', 'tojson', 'invocation_id', 'dbt_version', 'run_started_at'})
(maybe_keys) - potentail key hardcoded defined in test_context.py
frozenset({'debug'})

the base.py we are referencing has the print method added to BaseContext but if you go look at 1.0.latest branch which is what we should be pulling in for it doesn't.

dbt-labs/dbt-core#4701

@McKnight-42
Copy link
Contributor Author

McKnight-42 commented Mar 24, 2022

thoughts on why the unit test for tests/unit/test_redshift_adapter.py is failing: this change is missing in backport currently #58 due to #4512 in core

@McKnight-42
Copy link
Contributor Author

The docs_generate tests are failing due to testing against and looking for unique_key as a list changes made here #70

@leahwicz
Copy link
Contributor

@McKnight-42 I think you are on the right track here with the dev_requirements change!

@McKnight-42
Copy link
Contributor Author

Closed in favore of #89

@mikealfare mikealfare deleted the mcknight/backport-80 branch March 1, 2023 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants