-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
made change to test of str #4463
Conversation
The other alternative we considered was to have the status field of
|
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.
Let's just add a changelog entry before we merge.
@McKnight-42 this change will need to be backported to the |
@leahwicz would love to set up time to walk through that process |
* made change to test of str * changelog update
resolves #4433
Description
redefined status param of SQLQueryStatus to typecheck. string which passes on ._message value of AdapterResponse or the str value sent by adapter plugin.
Worked with Nate going through process of dealing with issues as they come in with minimal context, talked through several potential ways to solve the issue and even found some that would cause a bigger change than currently needed so had to take into account what might be worth time investment of task.
-- Co Author @nathaniel-may
Checklist
CHANGELOG.md
and added information about my change