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

Timeout limit for HTTP notifications #48

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Timeout limit for HTTP notifications #48

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/appier_extras/parts/admin/models/event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,11 @@ def _retry(cls, callable, count=3):
def notify_http(self, arguments={}):
cls = self.__class__
url = arguments.get("url", None)
timeout = arguments.get("timeout", None)
retries = arguments.get("retries", 3)
logger = appier.get_logger()
logger.debug("Running HTTP notification for '%s' ..." % url)
return cls._retry(lambda: appier.post(url, data_j=arguments), count=retries)
return cls._retry(lambda: appier.post(url, timeout=timeout, data_j=arguments), count=retries)
Comment on lines +185 to +189
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for HTTP notification timeout parameter

The timeout parameter lacks test coverage and documentation:

  • No test cases found for HTTP notification timeout scenarios in src/appier_extras/test/admin/models/event.py
  • No documentation found describing the timeout configuration in the documentation files
  • The feature is only implemented in the notify_http method without corresponding test coverage

Please add:

  1. Test cases covering timeout behavior
  2. Documentation for the timeout parameter in the HTTP notification configuration
  3. Example usage in the documentation
🔗 Analysis chain

Verify test coverage and documentation.

The timeout parameter addition should be reflected in:

  1. Unit tests covering timeout scenarios
  2. Documentation for HTTP notification configuration
  3. Example configurations demonstrating timeout usage
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for tests and documentation related to HTTP notifications

# Check for test files containing HTTP notification tests
echo "Searching for test files..."
fd -e py test | xargs rg -l "notify_http"

# Check for documentation files
echo "Searching for documentation files..."
fd -e md -e rst | xargs rg -l "notify_http"

Length of output: 279

Script:

#!/bin/bash
# Let's try a broader search to understand the HTTP notification implementation and testing

# Search for HTTP notification related code and tests
echo "Searching for HTTP notification patterns..."
rg -l "post.*timeout|timeout.*post|notify.*http" --type py

# Search for test files that might contain HTTP-related tests
echo "Searching for HTTP-related tests..."
rg -l "def test.*http|http.*test" --type py

# Look for configuration examples
echo "Searching for configuration examples..."
fd -e json -e yaml -e yml | xargs rg -l "notification|http|timeout"

# Check documentation more broadly
echo "Searching documentation more broadly..."
fd README -e md -e rst | xargs rg -l "notification|http|timeout"

Length of output: 5045

Script:

#!/bin/bash
# Let's examine the test file and documentation content specifically related to HTTP notifications

# Check the event model test file content
echo "Checking event model test content..."
rg -A 5 -B 5 "notify_http|post.*timeout" src/appier_extras/test/admin/models/event.py

# Check documentation content
echo "Checking documentation content..."
rg -A 5 -B 5 "notification|http.*timeout" doc/models.md doc/configuration.md README.md

# Look at the event model implementation
echo "Checking event model implementation..."
rg -A 5 -B 5 "notify_http" src/appier_extras/parts/admin/models/event.py

Length of output: 841


def notify_mailme(self, arguments={}):
cls = self.__class__
Expand Down