Create SECURITY.md #1
Conversation
* implement support for soft deletion of reactions * add tests for soft deletion * drop support for 3.7, add support for 3.12, drop lint commit msg * fix missing ' * add support for STREAM_REGION * make tests work with region * bump deps for python 3.12 * remove yassine from CODEOWNERS
* chore(release): v5.3.1 * empty to kickstart CI --------- Co-authored-by: github-actions <[email protected]> Co-authored-by: Jimmy Pettersson <[email protected]>
WalkthroughThe changes include updates to several files in the repository, primarily focusing on modifying the code owners, adjusting CI workflow configurations, enhancing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Server
User->>Client: Request to delete reaction
Client->>Server: DELETE /reactions/{reaction_id}
alt Soft delete
Server-->>Client: Reaction marked as soft deleted
else Hard delete
Server-->>Client: Reaction permanently deleted
end
User->>Client: Request to restore reaction
Client->>Server: PUT /reactions/{reaction_id}/restore
Server-->>Client: Reaction restored successfully
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (17)
SECURITY.md (3)
1-3: Excellent introduction, minor grammatical suggestion.The title and commitment statement effectively introduce the purpose of this document. However, consider adding a comma after "At Stream" for improved readability.
Suggested change:
-At Stream we are committed to the security of our Software. We appreciate your efforts in disclosing vulnerabilities responsibly and we will make every effort to acknowledge your contributions. +At Stream, we are committed to the security of our Software. We appreciate your efforts in disclosing vulnerabilities responsibly, and we will make every effort to acknowledge your contributions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~2-~2: Possible missing comma found.
Context: # Reporting a Vulnerability At Stream we are committed to the security of our...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~2-~2: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n disclosing vulnerabilities responsibly and we will make every effort to acknowledg...(COMMA_COMPOUND_SENTENCE)
4-7: Simplify email presentation format.The email address for reporting vulnerabilities is clearly provided. However, the current formatting using a code block is unnecessary for a single line of text.
Consider simplifying the format:
-Report security vulnerabilities at the following email address: -``` -[[email protected]](mailto:[email protected]) -``` +Report security vulnerabilities by email to [[email protected]](mailto:[email protected]).This change improves readability and removes the need for a language-less code block.
🧰 Tools
🪛 LanguageTool
[typographical] ~7-~7: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...m.io](mailto:[email protected]) ``` Alternatively it is also possible to open a new issue...(RB_LY_COMMA)
🪛 Markdownlint
5-5: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
8-10: Good alternative reporting method, minor grammatical suggestions.The inclusion of an alternative reporting method via GitHub issues is valuable. The acknowledgment statement provides reassurance to potential vulnerability reporters.
Consider these minor grammatical improvements:
-Alternatively it is also possible to open a new issue in the affected repository, tagging it with the `security` tag. +Alternatively, it is also possible to open a new issue in the affected repository, tagging it with the `security` tag. -A team member will acknowledge the vulnerability and will follow-up with more detailed information. A representative of the security team will be in touch if more information is needed. +A team member will acknowledge the vulnerability and will follow up with more detailed information. A representative of the security team will be in touch if more information is needed.🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The word “follow-up” is a noun. The verb is spelled with a space.
Context: ... acknowledge the vulnerability and will follow-up with more detailed information. A repre...(NOUN_VERB_CONFUSION)
.github/workflows/ci.yml (3)
19-19: Approve Python version update and suggest documentation update.The update to the Python version matrix is appropriate. Removing Python 3.7 (which reached end-of-life in June 2023) and adding Python 3.12 keeps the project current with modern Python versions.
Please ensure that the project's documentation (README, setup.py, etc.) is updated to reflect these new Python version requirements.
33-33: Approve linting condition update and suggest consideration.The update to run linting on Python 3.8 is consistent with the removal of Python 3.7 support.
Consider running the linting step on the latest supported Python version (3.12) instead of 3.8. This could help catch any potential issues or take advantage of new linting features in the latest Python version. If you decide to keep it on 3.8, please add a comment explaining why this specific version was chosen for linting.
Inconsistency Found in Commit Message Linter Configuration
The
ci.ymlfile contains a comment referencing the commit linter:fetch-depth: 0 # gives the commit linter access to previous commitsHowever, there is no actual commit message linter step present in the workflow. Please update the comment to reflect the current configuration or reinstate the commit message linter if it is still required.
🔗 Analysis chain
Line range hint
1-42: Request clarification on removal of commit message linter.The AI summary indicates that the commit message linter step has been removed from the workflow. While this change is not visible in the provided code snippet, it's an important modification to the CI process.
Could you please clarify the reasoning behind removing the commit message linter? If there are alternative methods in place to ensure commit message quality (e.g., pre-commit hooks, branch protection rules), it would be helpful to document this decision.
To verify the removal, you can run the following command:
If the command returns no results, it confirms that the commit message linter step has indeed been removed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify removal of commit message linter rg -i "commit message linter" .github/workflows/ci.ymlLength of output: 56
Script:
#!/bin/bash # Comprehensive search for commit message linter-related steps in ci.yml rg -iE 'commit.*linter|message.*lint|commit-message-check|commit-lint' .github/workflows/ci.ymlLength of output: 196
Script:
#!/bin/bash # Search for use of common commit linting actions or tools rg -iE 'actions/commitlint|commitlint|conventional-commits|cz-conventional-changelog' .github/workflows/ci.ymlLength of output: 226
Script:
#!/bin/bash # Check for invocation of commit message linting scripts rg -iE 'make\s+lint-commit|npm\s+run\s+lint-commit|yarn\s+lint-commit' .github/workflows/ci.ymlLength of output: 196
Script:
#!/bin/bash # Comprehensive search for commit message linter-related steps in ci.yml rg -i -e 'commit.*linter' -e 'message.*lint' -e 'commit-message-check' -e 'commit-lint' .github/workflows/ci.ymlLength of output: 188
Script:
#!/bin/bash # Search for use of common commit linting actions or tools rg -i -e 'actions/commitlint' -e 'commitlint' -e 'conventional-commits' -e 'cz-conventional-changelog' .github/workflows/ci.ymlLength of output: 121
Script:
#!/bin/bash # Check for invocation of commit message linting scripts rg -i -e 'make\s+lint-commit' -e 'npm\s+run\s+lint-commit' -e 'yarn\s+lint-commit' .github/workflows/ci.ymlLength of output: 109
stream/reactions/base.py (2)
26-27: LGTM! Consider adding documentation for the new parameter.The addition of the
softparameter with a default value ofFalseis a good way to implement a soft delete feature while maintaining backward compatibility.Consider adding a docstring to explain the purpose of the
softparameter and its behavior. For example:@abstractmethod def delete(self, reaction_id, soft=False): """ Delete a reaction. Args: reaction_id: The ID of the reaction to delete. soft (bool, optional): If True, perform a soft delete. Defaults to False. """ pass
29-31: LGTM! Consider adding documentation for the new method.The addition of the
restoremethod is a good complement to the soft delete feature.Consider adding a docstring to explain the purpose of the
restoremethod. For example:@abstractmethod def restore(self, reaction_id): """ Restore a soft-deleted reaction. Args: reaction_id: The ID of the reaction to restore. """ passstream/__init__.py (1)
34-36: Approved: Good addition for flexibility. Consider documentation and security.The new conditional check for
locationenhances flexibility by allowing it to be set via an environment variable. This is a good practice for configuration management.Consider the following suggestions:
- Update the function's docstring to reflect this new behavior.
- Add a note in the project's documentation about the
STREAM_REGIONenvironment variable.- Ensure that sensitive data (if any) is not accidentally exposed through environment variables in logs or error messages.
Here's a suggested update for the docstring:
def connect( api_key=None, api_secret=None, app_id=None, version="v1.0", timeout=3.0, location=None, base_url=None, use_async=False, ): """ Returns a Client object :param api_key: your api key or heroku url :param api_secret: the api secret :param app_id: the app id (used for listening to feed changes) :param use_async: flag to set AsyncClient :param location: the location to use. If not provided, it will use the STREAM_REGION environment variable. """stream/reactions/reaction.py (4)
45-52: LGTM! Consider adding a docstring for clarity.The implementation of soft deletion looks good. The
softparameter is correctly passed to the API.Consider adding a docstring to explain the
softparameter:def delete(self, reaction_id, soft=False): """ Delete a reaction. :param reaction_id: The ID of the reaction to delete. :param soft: If True, perform a soft delete. Default is False. :return: The response from the API. """ # ... rest of the method
54-57: LGTM! Consider adding a docstring for clarity.The implementation of the
restoremethod looks good. It correctly sends a PUT request to the appropriate endpoint.Consider adding a docstring to explain the method's purpose:
def restore(self, reaction_id): """ Restore a previously deleted reaction. :param reaction_id: The ID of the reaction to restore. :return: The response from the API. """ # ... rest of the method
135-142: LGTM! Consider adding a docstring for clarity.The implementation of soft deletion in the async version looks good. The
softparameter is correctly passed to the API, and the method usesawaitappropriately.Consider adding a docstring to explain the
softparameter:async def delete(self, reaction_id, soft=False): """ Asynchronously delete a reaction. :param reaction_id: The ID of the reaction to delete. :param soft: If True, perform a soft delete. Default is False. :return: The response from the API. """ # ... rest of the method
144-147: LGTM! Consider adding a docstring for clarity.The implementation of the async
restoremethod looks good. It correctly sends an asynchronous PUT request to the appropriate endpoint.Consider adding a docstring to explain the method's purpose:
async def restore(self, reaction_id): """ Asynchronously restore a previously deleted reaction. :param reaction_id: The ID of the reaction to restore. :return: The response from the API. """ # ... rest of the methodCHANGELOG.md (1)
5-6: Add details about changes in version 5.3.1The new changelog entry for version 5.3.1 is missing details about the changes included in this release. To maintain consistency with previous entries and provide valuable information to users, please add a brief description of the enhancements, features, or bug fixes introduced in this version.
Consider adding bullet points under the version entry to describe the changes, for example:
### [5.3.1](https://github.com/GetStream/stream-python/compare/v5.2.1...v5.3.1) (2023-10-25) * Added support for Python 3.11 * Introduced asynchronous support for non-blocking operations * Fixed issues related to tests and linting * Resolved problems with redirects, uniqueness, and deprecations🧰 Tools
🪛 Markdownlint
5-5: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
stream/tests/test_async_client.py (3)
1052-1058: Enhance test to verify hard deletionThe test correctly adds a reaction and attempts to delete it. However, it doesn't verify if the reaction is actually deleted after the operation. Consider adding an assertion to check if the reaction is no longer retrievable after deletion.
Here's a suggested improvement:
@pytest.mark.asyncio async def test_reaction_hard_delete(async_client): response = await async_client.reactions.add( "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" ) await async_client.reactions.delete(response["id"], soft=False) with pytest.raises(Exception): # Replace with the specific exception raised for non-existent reactions await async_client.reactions.get(response["id"])
1060-1066: Enhance test to verify soft deletion stateThe test correctly adds a reaction and attempts to soft delete it. However, it doesn't verify the state of the reaction after the soft delete operation. Consider adding assertions to check if the reaction is still retrievable but marked as deleted after the soft delete.
Here's a suggested improvement:
@pytest.mark.asyncio async def test_reaction_soft_delete(async_client): response = await async_client.reactions.add( "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" ) await async_client.reactions.delete(response["id"], soft=True) # Verify the reaction is still retrievable but marked as deleted deleted_reaction = await async_client.reactions.get(response["id"]) assert deleted_reaction["id"] == response["id"] assert "deleted_at" in deleted_reaction assert deleted_reaction["deleted_at"] is not None
1081-1088: Consider a more descriptive test nameThe test correctly checks for the expected exception when trying to restore a non-soft-deleted reaction. However, the test name "invalid_restore" might not accurately describe the specific scenario being tested.
Consider renaming the test to something more descriptive, such as:
@pytest.mark.asyncio async def test_reaction_restore_non_soft_deleted_raises_exception(async_client): # ... (rest of the test remains the same)This name more clearly indicates that the test is checking the behavior when attempting to restore a reaction that hasn't been soft deleted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
- .github/CODEOWNERS (1 hunks)
- .github/workflows/ci.yml (2 hunks)
- CHANGELOG.md (1 hunks)
- SECURITY.md (1 hunks)
- setup.py (1 hunks)
- stream/init.py (2 hunks)
- stream/reactions/base.py (1 hunks)
- stream/reactions/reaction.py (2 hunks)
- stream/tests/test_async_client.py (2 hunks)
- stream/tests/test_client.py (4 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
5-5: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
SECURITY.md
5-5: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
🪛 LanguageTool
SECURITY.md
[uncategorized] ~2-~2: Possible missing comma found.
Context: # Reporting a Vulnerability At Stream we are committed to the security of our...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~2-~2: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n disclosing vulnerabilities responsibly and we will make every effort to acknowledg...(COMMA_COMPOUND_SENTENCE)
[typographical] ~7-~7: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...m.io](mailto:[email protected]) ``` Alternatively it is also possible to open a new issue...(RB_LY_COMMA)
[grammar] ~10-~10: The word “follow-up” is a noun. The verb is spelled with a space.
Context: ... acknowledge the vulnerability and will follow-up with more detailed information. A repre...(NOUN_VERB_CONFUSION)
🔇 Additional comments (13)
.github/CODEOWNERS (1)
1-1: Verify the intentional removal of @yaziine as a code owner.The change reduces the number of code owners from three to two. While the syntax is correct, please confirm that:
- The removal of @yaziine as a code owner is intentional.
- The remaining code owners (@JimmyPettersson85 and @xernobyl) have sufficient expertise to cover all areas of the project.
- The team has considered the potential impact on the review process and workload distribution.
To ensure this change aligns with recent project decisions, you may want to check recent team discussions or announcements. Consider running the following command to search for relevant information:
This will help verify if there have been any recent discussions about changing code ownership that support this modification.
SECURITY.md (2)
12-16: Excellent reporting guidelines.The list of required information for vulnerability reports is clear, concise, and comprehensive. This section will effectively guide reporters in providing the necessary details for efficient vulnerability assessment and resolution.
1-16: Overall excellent SECURITY.md file.This SECURITY.md file successfully achieves the PR objective of introducing clear guidelines for reporting security vulnerabilities. It provides multiple reporting methods, sets expectations for acknowledgment and follow-up, and outlines the necessary information for effective vulnerability reports. The minor suggested improvements will further enhance its clarity and professionalism.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~2-~2: Possible missing comma found.
Context: # Reporting a Vulnerability At Stream we are committed to the security of our...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~2-~2: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n disclosing vulnerabilities responsibly and we will make every effort to acknowledg...(COMMA_COMPOUND_SENTENCE)
[typographical] ~7-~7: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...m.io](mailto:[email protected]) ``` Alternatively it is also possible to open a new issue...(RB_LY_COMMA)
[grammar] ~10-~10: The word “follow-up” is a noun. The verb is spelled with a space.
Context: ... acknowledge the vulnerability and will follow-up with more detailed information. A repre...(NOUN_VERB_CONFUSION)
🪛 Markdownlint
5-5: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
stream/reactions/base.py (1)
26-31: Summary of changes and their impactThe modifications to the
AbstractReactionsclass enhance the functionality of the reactions system by introducing soft delete and restore capabilities. These changes are well-structured and maintain backward compatibility. Key points:
- The
deletemethod now supports soft deletion through an optionalsoftparameter.- A new
restoremethod has been added to complement the soft delete feature.- These changes will require updates to concrete implementations of
AbstractReactionsand associated tests.Overall, these changes provide a more flexible reaction management system, allowing for temporary removal and restoration of reactions.
setup.py (4)
11-11:⚠️ Potential issueCaution:
aiohttpdependency updated to beta versionThe minimum version of
aiohttphas been changed from 3.8.4 to 3.9.0b0, which is a beta version. Using beta versions in production code can lead to instability and unexpected behavior.Consider the following alternatives:
- Use the latest stable version instead of the beta version.
- If features from the beta version are required, ensure thorough testing and consider adding an upper bound to prevent automatic updates to potentially unstable future beta or release candidates.
Example of a more stable version requirement:
"aiohttp>=3.8.4,<3.9.0"Or if the beta features are necessary:
"aiohttp>=3.9.0b0,<3.9.0"To assess the impact of this change, please run the following commands:
#!/bin/bash # Description: Check for compatibility issues with the updated aiohttp library # Test: Compare available versions and run tests pip install aiohttp==3.9.0b0 pip freeze | grep aiohttp pytest tests/ # Test: Check for any deprecation warnings or new features used grep -R "import aiohttp" . grep -R "from aiohttp" .Please review the output carefully for any deprecation warnings or errors related to
aiohttpusage.
10-10: Approved with suggestion:pytzdependency updateThe minimum version of
pytzhas been increased from 2022.7.1 to 2023.3.post1, which is good for incorporating recent timezone data updates and potential bug fixes.Consider adding an upper bound to the
pytzdependency to prevent potential future compatibility issues. For example:"pytz>=2023.3.post1,<2024"To ensure compatibility, please verify that the project works correctly with this new version. You can run the following command to check for any potential issues:
#!/bin/bash # Description: Check for compatibility issues with the updated pytz library # Test: Run tests with the new pytz version pip install pytz==2023.3.post1 && pytest tests/
8-8: Approved:requestsdependency updateThe minimum version of
requestshas been increased from 2.28.0 to 2.31.0, which is good for incorporating recent improvements and security fixes. The upper bound is maintained, preventing potential breaking changes.To ensure compatibility, please verify that the project works correctly with this new version. You can run the following command to check for any potential issues:
9-9: Approved:pyjwtdependency updateThe minimum version of
pyjwthas been increased from 2.6.0 to 2.8.0, which is good for incorporating recent improvements and security fixes. The upper bound is maintained, preventing potential breaking changes.To ensure compatibility, please verify that the project works correctly with this new version. You can run the following command to check for any potential issues:
stream/reactions/reaction.py (1)
Line range hint
1-147: Overall, the changes look good and enhance the functionality of the Reactions system.The modifications to both
ReactionsandAsyncReactionsclasses successfully implement soft deletion and restoration of reactions. These changes provide more flexibility in managing reactions, allowing for temporary removal and recovery of reaction data.Key points:
- Soft deletion is now possible through the
deletemethod's newsoftparameter.- A new
restoremethod has been added to both classes for recovering soft-deleted reactions.- The changes are consistent across both synchronous and asynchronous implementations.
The implementation is clean and follows the existing code structure. Consider adding docstrings to the new and modified methods for better documentation.
CHANGELOG.md (1)
5-5: Maintain consistent heading levels in the changelogThe static analysis tool (Markdownlint) has flagged an issue with the heading level, suggesting that it should be an h2 (##) instead of an h3 (###). However, the current h3 heading is consistent with the established format of this changelog.
Recommendation: Ignore this static analysis hint to maintain consistency throughout the changelog. If desired, you can add a Markdownlint configuration file to suppress this specific rule for the CHANGELOG.md file.
🧰 Tools
🪛 Markdownlint
5-5: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
stream/tests/test_async_client.py (2)
1068-1079: Well-implemented test for soft delete and restoreThis test thoroughly covers the soft delete and restore functionality for reactions. It correctly verifies the state of the reaction after soft deletion and restoration by checking the presence and absence of the 'deleted_at' field.
1052-1088: Overall assessment of new reaction testsThe new tests for reaction handling (hard delete, soft delete, restore, and invalid restore) are valuable additions to the test suite. They cover important scenarios and help ensure the correct functionality of the reaction system. The suggested improvements will enhance the tests' effectiveness by verifying deletion states more thoroughly and improving naming clarity.
Here's a summary of the recommendations:
- Enhance
test_reaction_hard_deleteto verify that the reaction is not retrievable after deletion.- Improve
test_reaction_soft_deleteto check the state of the reaction after soft deletion.- The
test_reaction_soft_delete_and_restorefunction is well-implemented and covers the necessary steps.- Rename
test_reaction_invalid_restoreto more accurately describe the specific scenario being tested.Implementing these suggestions will result in a more robust and clear test suite for reaction handling.
stream/tests/test_client.py (1)
1454-1464: Good implementation of reaction soft delete and restore testThe
test_reaction_soft_delete_and_restoremethod correctly tests the soft deletion and restoration of a reaction, with appropriate assertions to verify thedeleted_atattribute before and after restoration.
| def delete(self, reaction_id, soft=False): | ||
| pass | ||
|
|
||
| @abstractmethod | ||
| def restore(self, reaction_id): | ||
| pass |
There was a problem hiding this comment.
💡 Codebase verification
Add tests for restore and soft delete functionalities.
Currently, there are no tests covering the restore method and the soft parameter in the delete method. To ensure these functionalities work as intended, please add the following tests:
-
restoreMethod:- Verify that the method correctly restores a previously deleted reaction.
- Ensure appropriate handling of invalid
reaction_idinputs.
-
softParameter indeleteMethod:- Test both scenarios where
softisTrueandFalse. - Confirm that soft deletes mark reactions appropriately without permanent removal.
- Ensure that the behavior aligns with the application's requirements.
- Test both scenarios where
Implementing these tests will enhance the reliability and maintainability of the AbstractReactions implementations.
🔗 Analysis chain
Ensure concrete implementations and tests are updated.
The changes to the AbstractReactions class will require updates to concrete implementations and tests:
-
The
BaseReactionsclass in this file, and any other concrete implementations ofAbstractReactionsin the codebase, will need to:- Implement the new
restoremethod. - Update the
deletemethod to handle thesoftparameter.
- Implement the new
-
Existing tests should be updated, and new tests should be added to cover:
- The soft delete functionality.
- The restore functionality.
To help identify the classes that need to be updated, you can run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all Python files that likely contain implementations of AbstractReactions
echo "Files potentially containing AbstractReactions implementations:"
rg --type python -l "class \w+\(.*AbstractReactions.*\):"
# Find all test files related to reactions
echo -e "\nTest files related to reactions:"
fd -e py test.*reaction
Length of output: 336
Script:
#!/bin/bash
# Corrected script to find all Python files that likely contain implementations of AbstractReactions
echo "Files potentially containing AbstractReactions implementations:"
rg --type py -l "class \w+\(.*AbstractReactions.*\):"
# Corrected script to find all test files related to reactions with a more flexible pattern
echo -e "\nTest files related to reactions:"
fd -e py -g "*test*reaction*"
Length of output: 330
Script:
#!/bin/bash
# Find all Python test files that mention 'reaction' in their filenames
echo "All Python test files related to reactions:"
rg --type py -l "reaction" <<< "$(fd -e py)"
# Additionally, search within test files for mentions of 'restore' or 'soft delete'
echo -e "\nSearching within test files for 'restore' or 'soft delete' functionalities:"
rg --type py "(def restore|soft\s*delete)" -- test/
Length of output: 422
Script:
#!/bin/bash
# Find all Python test files that include 'test' in their filenames, regardless of their location
echo "All Python test files related to reactions:"
fd -e py -g "*test*.py"
# Capture the list of test files
TEST_FILES=$(fd -e py -g "*test*.py")
# Check if any test files were found
if [ -z "$TEST_FILES" ]; then
echo -e "\nNo test files related to reactions were found."
else
echo -e "\nSearching within test files for 'restore' or 'soft delete' functionalities:"
# Search within the found test files for 'restore' or 'soft delete'
rg --type py "(def restore|soft\s*delete)" $TEST_FILES
fi
Length of output: 751
| @@ -5,7 +5,7 @@ | |||
| __copyright__ = "Copyright 2022, Stream.io, Inc" | |||
| __credits__ = ["Thierry Schellenbach, mellowmorning.com, @tschellenbach"] | |||
| __license__ = "BSD-3-Clause" | |||
| __version__ = "5.2.1" | |||
| __version__ = "5.3.1" | |||
There was a problem hiding this comment.
💡 Codebase verification
CHANGELOG.md is not updated to reflect the new version.
stream/__init__.pyis updated to version 5.3.1.CHANGELOG.mdstill lists version 5.2.0.
Please update CHANGELOG.md to include the new version 5.3.1.
🔗 Analysis chain
Version update looks good. Verify consistency across the project.
The version bump from 5.2.1 to 5.3.1 is appropriate given the new features mentioned in the summary.
Please ensure this version update is consistent across the project. Run the following script to check:
Version inconsistency detected.
- The
__init__.pyfile is updated to version 5.3.1, but theCHANGELOG.mdreflects version 5.2.0. - Unable to verify the version in
setup.pydue to a file type recognition issue.
Please run the following script to check the version in setup.py without specifying the file type:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version consistency across key files
# Test: Check version in setup.py
echo "Version in setup.py:"
rg --type python 'version\s*=' setup.py
# Test: Check for version update in CHANGELOG.md
echo "Latest version in CHANGELOG.md:"
rg '^## \[' CHANGELOG.md | head -n 1
# Test: Check version in __init__.py (for confirmation)
echo "Version in __init__.py:"
rg '__version__\s*=' stream/__init__.py
Length of output: 448
Script:
#
#!/bin/bash
# Description: Check version consistency in setup.py without type specification
echo "Version in setup.py:"
rg 'version\s*=' setup.py
Length of output: 103
| def test_reaction_invalid_restore(self): | ||
| response = self.c.reactions.add( | ||
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | ||
| ) | ||
| self.assertRaises( | ||
| DoesNotExistException, lambda: self.c.reactions.restore(response["id"]) | ||
| ) | ||
|
|
There was a problem hiding this comment.
Verify correct exception is raised when restoring non-deleted reaction
In test_reaction_invalid_restore, the test attempts to restore a reaction that has not been deleted and expects a DoesNotExistException. Please confirm that attempting to restore a non-deleted reaction raises the DoesNotExistException. If this is not the expected behavior, consider updating the test or handling a different exception.
| def test_reaction_soft_delete(self): | ||
| response = self.c.reactions.add( | ||
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | ||
| ) | ||
| self.c.reactions.delete(response["id"], soft=True) | ||
|
|
There was a problem hiding this comment.
Add assertions to verify reaction soft deletion
In test_reaction_soft_delete, after performing a soft delete, there are no assertions to verify that the reaction is marked as deleted. Adding assertions to check the deleted_at attribute can ensure that the soft delete functionality is working as expected.
Apply this diff to add the assertion:
def test_reaction_soft_delete(self):
response = self.c.reactions.add(
"like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike"
)
self.c.reactions.delete(response["id"], soft=True)
+ r1 = self.c.reactions.get(response["id"])
+ self.assertIsNotNone(r1.get("deleted_at"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_reaction_soft_delete(self): | |
| response = self.c.reactions.add( | |
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | |
| ) | |
| self.c.reactions.delete(response["id"], soft=True) | |
| def test_reaction_soft_delete(self): | |
| response = self.c.reactions.add( | |
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | |
| ) | |
| self.c.reactions.delete(response["id"], soft=True) | |
| r1 = self.c.reactions.get(response["id"]) | |
| self.assertIsNotNone(r1.get("deleted_at")) |
| def test_reaction_hard_delete(self): | ||
| response = self.c.reactions.add( | ||
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | ||
| ) | ||
| self.c.reactions.delete(response["id"], soft=False) | ||
|
|
There was a problem hiding this comment.
Add assertions to verify reaction hard deletion
The method test_reaction_hard_delete performs a hard delete on a reaction but does not include assertions to verify that the reaction has been successfully deleted. Consider adding assertions to confirm that the reaction no longer exists after deletion.
Apply this diff to add the assertion:
def test_reaction_hard_delete(self):
response = self.c.reactions.add(
"like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike"
)
self.c.reactions.delete(response["id"], soft=False)
+ self.assertRaises(
+ DoesNotExistException, lambda: self.c.reactions.get(response["id"])
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_reaction_hard_delete(self): | |
| response = self.c.reactions.add( | |
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | |
| ) | |
| self.c.reactions.delete(response["id"], soft=False) | |
| def test_reaction_hard_delete(self): | |
| response = self.c.reactions.add( | |
| "like", "54a60c1e-4ee3-494b-a1e3-50c06acb5ed4", "mike" | |
| ) | |
| self.c.reactions.delete(response["id"], soft=False) | |
| self.assertRaises( | |
| DoesNotExistException, lambda: self.c.reactions.get(response["id"]) | |
| ) |
Summary:
Submitter checklist:
CHANGELOGupdated or N/AMerger checklist:
Dependencies:
Summary by CodeRabbit
Release Notes for Version 5.3.1
New Features
Improvements
Bug Fixes