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

added fix for snowflake operator to accept authenticator and make parameters optional based on load_type #92

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

ckonuganti
Copy link
Contributor

Description

Added code to handle authenticator issue posted in issue #85
Added snowflake library installation to avoid initialization failures for brickflow plugins
enhanced the code to accept parameters based on load_type instead of making every parameter mandatory.

Related Issue

fix #85

Motivation and Context

At this point code is failing for user id's which require extra authentication mechanism which is blocking multiple brickflow consumers. As we added snowflake operator to initialize with brickflow_plugins it is looking for snowflake library and causing issues to some customers, fixing this issue. Current version of the operator expects optional parameters like sf_cluster_keys, incremental_filter, sf_grantee_roles even when they are not required for customers and causing issues.

How Has This Been Tested?

Created a wheel file , workflow and tested with user id that require and doesn't require extra authentication. ran multiple test cases with different combination of input params. created a notebook with operator code and ran multiple manual unit tests.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asingamaneni
Copy link
Collaborator

@ckonuganti can you please post some screenshots of this working

@ckonuganti
Copy link
Contributor Author

all_test_cases_uc_2_sf_test
Ad
task_with_libraries_uc_2_sf_testing
ding workflow screenshots with test cases.

Copy link
Collaborator

@asingamaneni asingamaneni left a comment

Choose a reason for hiding this comment

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

LGTM.

@asingamaneni asingamaneni merged commit 5a4fdfc into Nike-Inc:main Feb 15, 2024
2 checks passed
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.

[BUG] UcToSnowflakeOperator throws Incorrect Username and Password error
2 participants