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

Snow 1313805 fix connection test #972

Merged
merged 14 commits into from
Apr 9, 2024

Conversation

sfc-gh-pczajka
Copy link
Contributor

@sfc-gh-pczajka sfc-gh-pczajka commented Apr 8, 2024

Pre-review checklist

  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added or updated automated unit tests to verify correctness of my new code.
  • I've added or updated integration tests to verify correctness of my new code.
  • I've confirmed that my changes are working by executing CLI's commands manually.
  • I've confirmed that my changes are up-to-date with the target branch.
  • I've described my changes in the release notes.
  • I've described my changes in the section below.

Changes description

Fix overriding "schema" parameter in snow connection test

@sfc-gh-pczajka sfc-gh-pczajka requested a review from a team as a code owner April 8, 2024 14:23
@sfc-gh-pczajka sfc-gh-pczajka changed the base branch from main to SNOW-1292755-fix-dashes-in-connection-specification April 8, 2024 14:23
Comment on lines 20 to 24
@contextmanager
def set_env(env_name: str, value: str):
os.environ[env_name] = value
yield
del os.environ[env_name]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use mock.patch.dict(os.environ)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #971

@sfc-gh-pczajka sfc-gh-pczajka force-pushed the SNOW-1313805-fix-connection-test branch from ba4c344 to 3203d36 Compare April 8, 2024 15:19
mock.call(object_type=ObjectType.ROLE, name=f'"{conn.role}"'),
mock.call(object_type=ObjectType.DATABASE, name=f'"{conn.database}"'),
mock.call(object_type=ObjectType.SCHEMA, name=f'"{conn.schema}"'),
mock.call(object_type=ObjectType.WAREHOUSE, name=f'"{conn.warehouse}"'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How it was passing previously?

Copy link
Contributor Author

@sfc-gh-pczajka sfc-gh-pczajka Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't :] - fized in #971

Base automatically changed from SNOW-1292755-fix-dashes-in-connection-specification to main April 9, 2024 11:39
@sfc-gh-pczajka sfc-gh-pczajka merged commit af17da3 into main Apr 9, 2024
11 checks passed
@sfc-gh-pczajka sfc-gh-pczajka deleted the SNOW-1313805-fix-connection-test branch April 9, 2024 12:46
sfc-gh-sichen pushed a commit that referenced this pull request Oct 17, 2024
* fix schema override in connections test
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.

2 participants