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.

Sign up for GitHub

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

Jump to bottom

Fix reading of token_path option #12366

Merged
merged 2 commits into from
Jun 15, 2022
Merged

Fix reading of token_path option #12366

merged 2 commits into from
Jun 15, 2022

Conversation

fanny-jiang
Copy link
Contributor

@fanny-jiang fanny-jiang commented Jun 14, 2022 •
edited

What does this PR do?

The open method with binary mode, does not accept an encoding argument. Removes the unnecessary encoding argument.

Motivation

Support case
Fixes the error: ValueError: binary mode doesn't take an encoding argument

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@fanny-jiang fanny-jiang requested a review from a team as a code owner June 14, 2022 16:20
@codecov
Copy link

codecov bot commented Jun 14, 2022 •
edited

Codecov Report

Merging #12366 (8470fc8) into master (45a159c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag Coverage Δ
snowflake 94.91% <100.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

steveny91
steveny91 previously approved these changes Jun 14, 2022
View reviewed changes
ofek
ofek previously approved these changes Jun 14, 2022
View reviewed changes
@ofek ofek changed the title Fix read token_path Fix reading of token_path option Jun 14, 2022
@fanny-jiang fanny-jiang dismissed stale reviews from ofek and steveny91 via 8470fc8 June 15, 2022 07:29
@fanny-jiang fanny-jiang merged commit f17ba35 into master Jun 15, 2022
@fanny-jiang fanny-jiang deleted the fanny/sf-token branch June 15, 2022 08:54
github-actions bot pushed a commit that referenced this pull request Jun 15, 2022
@fanny-jiang
Fix reading of token_path option (#12366)
65597ea 
* AGENT-8213 fix read token

* remove unnecessary dd_run_check f17ba35
@sarah-witt sarah-witt mentioned this pull request Jul 5, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveny91 steveny91 steveny91 approved these changes

@ofek ofek ofek left review comments

Assignees
No one assigned
Labels
integration/snowflake
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fanny-jiang @ofek @steveny91