Skip to content

CBAPI - 3877 - Change the attrdict builtin#314

Merged
dganev-cb merged 2 commits intomasterfrom
CBAPI-3877-fix-attrdict
Jun 16, 2022
Merged

CBAPI - 3877 - Change the attrdict builtin#314
dganev-cb merged 2 commits intomasterfrom
CBAPI-3877-fix-attrdict

Conversation

@dganev-cb
Copy link
Copy Markdown
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Tests have been added that prove the fix is effective or that the feature works.
  • New and existing tests pass locally with the changes.
  • Code follows the style guidelines of this project (PEP8, clean code).
  • Linter has passed locally and any fixes were made for failures.
  • A self-review of the code has been done.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes (not tied to bugs/features)
  • Other (please describe):

What is the ticket or issue number?

CBAPI-3877 | #298

Pull Request Description

I've only changed the setup packages attrdict to attrdict3 it seems complex to change the whole auth file and since that there are no tests I've decided to go to the easies solution.

Does this introduce a breaking change?

  • Yes
  • No

@dganev-cb dganev-cb changed the title Change the attrdict builtin into attrdict3 CBAPI-3877 - Change the attrdict builtin into attrdict3 Jun 14, 2022
@dganev-cb dganev-cb changed the title CBAPI-3877 - Change the attrdict builtin into attrdict3 CBAPI - 3877 - Change the attrdict builtin into attrdict3 Jun 14, 2022
Copy link
Copy Markdown
Contributor

@avanbrunt-cb avanbrunt-cb left a comment

Choose a reason for hiding this comment

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

Please don't use attrdict3 as that is out of our control and the behavior we need can be easily replicated. See carbonblack/carbon-black-cloud-sdk-python#5

@dganev-cb dganev-cb requested a review from avanbrunt-cb June 15, 2022 06:54
@avanbrunt-cb avanbrunt-cb changed the title CBAPI - 3877 - Change the attrdict builtin into attrdict3 CBAPI - 3877 - Change the attrdict builtin Jun 15, 2022
Copy link
Copy Markdown
Contributor

@avanbrunt-cb avanbrunt-cb left a comment

Choose a reason for hiding this comment

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

Approved

@dganev-cb
Copy link
Copy Markdown
Contributor Author

@avanbrunt-cb Should I merge this and release it as a new version?

@dganev-cb dganev-cb merged commit 8418dd5 into master Jun 16, 2022
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.

3 participants