Add Community Snapshot Videos Command #2948
Conversation
github-actions
bot
added
backend
docker
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a full OWASP community snapshot video pipeline: new Slide/SlideBuilder/VideoGenerator, ElevenLabs TTS wrapper, HTML slide templates, a Django management command and Makefile target, new Dockerfile for video, poetry "video" group, tests, cspell entries, and .gitignore update. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
backend/docker/Dockerfile.local (1)
47-53: Update the Alpine version reference in the comment.Line 47 references Alpine 3.17, but the base image
python:3.13.7-alpinelikely uses a much newer Alpine version (3.20 or 3.21). While the WeasyPrint dependencies should still be correct, the comment may be misleading.Apply this diff to make the comment version-agnostic:
-# WeasyPrint dependencies: https://doc.courtbouillon.org/weasyprint/stable/first_steps.html#alpine-3-17 +# WeasyPrint dependencies: https://doc.courtbouillon.org/weasyprint/stable/first_steps.html RUN --mount=type=cache,target=${APK_CACHE_DIR} \Alternatively, verify the actual Alpine version and update the comment accordingly.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
40-40: Consider using logger instead of print statements.While
print()statements are acceptable in Django management commands (and explicitly allowed by ruff configuration line 99), using the module-levelloggerwould provide better control over log levels and formatting for production use.Example refactor:
- print(f"Rendering slide image: {self.name}") + logger.info(f"Rendering slide image: {self.name}")Apply similar changes to all print statements. However, if you prefer the current approach for command output, it's acceptable to keep the print statements.
Also applies to: 54-54, 58-58, 69-69, 92-92, 95-95
71-74: Reminder: Implement audio generation.The TODO indicates audio generation is not yet implemented. The hardcoded test audio path will need to be replaced with actual audio generation logic (likely using OpenAI TTS API).
Do you want me to help generate a skeleton implementation for audio generation using OpenAI's text-to-speech API, or open a separate issue to track this task?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
backend/apps/owasp/Makefile(1 hunks)backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)backend/apps/owasp/templates/video/base.html(1 hunks)backend/apps/owasp/templates/video/slides/intro.html(1 hunks)backend/docker/Dockerfile.local(1 hunks)backend/pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
backend/apps/common/open_ai.py (4)
OpenAi(13-105)set_prompt(64-76)set_input(36-48)complete(78-105)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)
🪛 GitHub Actions: Run CI/CD
backend/docker/Dockerfile.local
[error] 51-51: Unknown word (libgobject) detected by spell checker.
[error] 51-51: Unknown word (libpango) detected by spell checker.
[error] 51-51: Unknown word (libharfbuzz) detected by spell checker.
[error] 52-52: Unknown word (libharfbuzz) detected by spell checker.
[error] 52-52: Unknown word (libpangoft) detected by spell checker.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
[error] 10-10: Unknown word (pdfium) detected by spell checker.
[error] 44-44: Unknown word (pdfium) detected by spell checker.
[error] 47-47: Unknown word (pil) detected by spell checker.
[error] 50-50: Unknown word (pil) detected by spell checker.
[error] 73-73: Unknown word (rudransh) detected by spell checker.
[error] 73-73: Unknown word (shrivastava) detected by spell checker.
[error] 86-86: Unknown word (acodec) detected by spell checker.
[error] 87-87: Unknown word (vcodec) detected by spell checker.
[error] 87-87: Unknown word (libx) detected by spell checker.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
backend/apps/owasp/Makefile (1)
25-27: LGTM!The new Makefile target follows the existing pattern and is correctly structured. The
$(snapshot_key)variable will need to be provided by the caller, consistent with other targets likeowasp-create-project-metadata-file.backend/apps/owasp/templates/video/slides/intro.html (1)
1-7: LGTM!The intro slide template correctly extends the base template and uses the context variables provided by
SlideBuilder.create_intro_slide(). The structure is clean and appropriate for the video generation workflow.backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
98-119: LGTM!The
SlideBuilderclass is well-structured and correctly creates slide instances with appropriate context and paths. The date formatting and file naming conventions are clear and consistent.
122-138: LGTM!The
Generatorclass provides a clean abstraction for managing slides and orchestrating the video generation pipeline. The sequential processing ingenerate_video()is appropriate for the PoC stage.
164-185: LGTM with a note on query pattern.The command handler correctly validates input, queries for a completed snapshot, and orchestrates the video generation workflow. The use of
filter().first()with case-insensitive matching is safer thanget()for handling user input, as it gracefully returnsNonerather than raisingDoesNotExist.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
57-74: Handle transcript generation failures more robustly.The method catches exceptions but doesn't re-raise them, and it doesn't handle the case where
open_ai.complete()returnsNone(as documented in the OpenAi class). If transcript generation fails,self.transcriptmight not be set, causing downstream failures when it's accessed.Apply this diff to handle failures properly:
def generate_transcript(self, open_ai: OpenAi): """Generate a transcript.""" logger.info("Prompt: %s", self.input) prompt = """ You are a scriptwriter for a tech presentation. Your task is to generate a script for a presentation slide. The script should be simple and direct. Do not add any extra words, introduction or conclusion. """ open_ai.set_prompt(prompt) open_ai.set_input(self.input) try: self.transcript = open_ai.complete() + if self.transcript is None: + logger.error("Failed to generate transcript for %s", self.name) + raise RuntimeError(f"OpenAI API failed to generate transcript for {self.name}") logger.info("Generated slide transcript: %s", self.transcript) except Exception: logger.exception("Error generating transcript for %s:", self.name) + raise
🧹 Nitpick comments (4)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
75-78: Placeholder audio generation needs implementation.The hardcoded audio path will cause all slides to share the same audio file, leading to incorrect video assembly when multiple slides are processed.
Do you want me to generate a skeleton implementation that creates unique audio files per slide, or would you prefer to open an issue to track this TODO?
104-107: Add docstring for consistency.The
__init__method is missing a docstring, while other methods have them.Apply this diff to add the docstring:
def __init__(self, snapshot, output_dir): - """Initialize SlideBuilder.""" + """Initialize SlideBuilder. + + Args: + snapshot: The snapshot object containing data for video generation. + output_dir: Path to the directory where output files will be saved. + + """ self.snapshot = snapshot self.output_dir = output_dir
128-135: Consider adding type hints for better code clarity.The
append_slidemethod and__init__would benefit from parameter type hints matching the rest of the codebase.Apply this diff to improve type annotations:
- def __init__(self): - """Initialize Video Generator.""" + def __init__(self) -> None: + """Initialize Video Generator. + + Creates an empty slide list and initializes the OpenAI client + for transcript generation. + + """ self.slides: list[Slide] = [] self.open_ai = OpenAi() - def append_slide(self, slide: Slide): - """Append a slide to list.""" + def append_slide(self, slide: Slide) -> None: + """Append a slide to list. + + Args: + slide: The Slide instance to add to the generation queue. + + """ self.slides.append(slide)backend/docker/Dockerfile.local (1)
49-53: Alpine package availability confirmed; consider version pinning for reproducible builds.All specified packages are available in Alpine Linux. The WeasyPrint documentation reference is current, and ffmpeg can be installed via
apk add ffmpegon Alpine Linux. Theso:prefix notation for shared libraries is valid Alpine package syntax.For more reproducible builds, consider pinning package versions:
ffmpeg=~6.1(or a specific stable version)ttf-freefont=20120212(or current stable)This prevents unintended package updates between builds.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)backend/apps/owasp/templates/video/base.html(1 hunks)backend/docker/Dockerfile.local(1 hunks)cspell/custom-dict.txt(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/templates/video/base.html
🧰 Additional context used
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
backend/apps/common/open_ai.py (4)
OpenAi(13-105)set_prompt(64-76)set_input(36-48)complete(78-105)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (8)
cspell/custom-dict.txt (1)
38-38: Alphabetical ordering and relevance confirmed.All nine new dictionary entries are correctly positioned in alphabetical order and directly correspond to the video generation dependencies being introduced (FFmpeg codecs, WeasyPrint rendering libraries, Pillow, and pypdfium2). The additions are appropriate and necessary to prevent false spell-check warnings for these technical identifiers.
Also applies to: 81-85, 110-110, 114-114, 141-141
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (7)
1-18: LGTM!The imports are well-organized and all appear necessary for the video generation workflow.
80-100: LGTM!The video generation logic correctly handles ffmpeg operations with proper error logging and exception propagation. The codec parameters are appropriate for standard video output.
109-124: LGTM!The intro slide creation correctly extracts and formats snapshot data, building a properly configured Slide instance.
137-143: LGTM!The sequential processing of slides (render → transcript → audio → video) is a clear and appropriate workflow for video generation.
169-176: LGTM!Environment variable setup in the command handler is appropriate, and the output directory creation logic is sound.
178-184: Consider more explicit error handling for snapshot lookup.Using
filter().first()with a None check is valid but less explicit than using a try-except withSnapshot.DoesNotExist. The current pattern matches the code snippet fromsnapshot.py, so consistency is maintained.
186-193: LGTM!The video generation workflow correctly initializes the builder and generator, creates the intro slide, and triggers the generation pipeline.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)backend/apps/owasp/templates/video/base.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/templates/video/base.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
backend/apps/common/open_ai.py (4)
OpenAi(13-105)set_prompt(64-76)set_input(36-48)complete(78-105)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
76-81: Add explicit None check for API response.The
try-exceptblock won't catch failures ifopen_ai.complete()returnsNoneinstead of raising an exception. According to theOpenAiclass implementation, thecomplete()method returnsNoneon error.🔎 Add explicit None check:
open_ai.set_prompt(prompt) open_ai.set_input(self.input) - try: - self.transcript = open_ai.complete() - print("Generated slide transcript:", self.transcript) - except Exception: - logger.exception("Error generating transcript for %s", self.name) - raise + self.transcript = open_ai.complete() + if self.transcript is None: + msg = f"Failed to generate transcript for {self.name}" + logger.error(msg) + raise RuntimeError(msg) + + print("Generated slide transcript:", self.transcript)
🧹 Nitpick comments (4)
backend/apps/common/eleven_labs.py (1)
43-55: Validate text parameter in set_text().The method accepts any text value without validation. Empty or
Nonetext will cause API failures later. Consider adding validation to fail fast with a clear error message.🔎 Add parameter validation:
def set_text(self, text: str) -> ElevenLabs: """Set the text to convert to speech. Args: text (str): The text content. Returns: ElevenLabs: The current instance. """ + if not text: + msg = "Text cannot be empty" + raise ValueError(msg) + self.text = text return selfbackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
42-42: Replace print() with appropriate output methods.Based on learnings, Django management commands should use
self.stdout.write()for user-facing output. However, since these helper classes don't have access to the command's stdout, consider:
- For the
Command.handle()method (line 212): useself.stdout.write()instead ofprint()- For helper classes (
Slide,SlideBuilder,Generator): uselogger.info()for structured loggingBased on learnings, Django management commands should prefer
self.stdout.write()overprint()for user-facing command output.🔎 Example changes:
In the Command class:
- print("Generating video for snapshot:", snapshot_key) + self.stdout.write(f"Generating video for snapshot: {snapshot_key}")In helper classes like Slide:
- print("Rendering slide image") + logger.info("Rendering slide image: %s", self.name)- print("Saved image to path:", self.image_output_path) + logger.info("Saved %s image to path: %s", self.name, self.image_output_path)Also applies to: 54-54, 66-66, 78-78, 99-99, 120-120, 135-135, 212-212
122-124: Remove redundant logging before re-raising exception.Logging immediately before re-raising an exception creates redundant logging. Let the caller handle logging the exception appropriately.
Based on learnings, ruff linting rules discourage logging immediately before raising exceptions.
🔎 Remove the logging call:
except ffmpeg.Error: - logger.exception("Error generating video for %s:", self.name) raise
128-128: Add type hint for snapshot parameter.The
snapshotparameter is missing a type hint. Based on the import at line 18, it should be typed asSnapshot.🔎 Add type hint:
- def __init__(self, snapshot, output_dir: Path) -> None: + def __init__(self, snapshot: Snapshot, output_dir: Path) -> None: """Initialize SlideBuilder."""
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
backend/.env.example(1 hunks)backend/apps/common/eleven_labs.py(1 hunks)backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)backend/pyproject.toml(1 hunks)backend/settings/base.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/pyproject.toml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🪛 GitHub Actions: Run CI/CD
backend/settings/base.py
[error] 219-219: CSpell: Unknown word 'ELEVENLABS'.
[error] 219-219: CSpell: Unknown word 'ELEVENLABS'.
backend/apps/common/eleven_labs.py
[error] 9-9: CSpell: Unknown word 'elevenlabs'.
[error] 22-22: CSpell: Unknown word 'MSU'.
[error] 35-35: CSpell: Unknown word 'ELEVENLABS'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL (javascript-typescript)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
53-53: Consider replacing print statements with structured logging.Multiple
print()statements throughout the Slide class should use structured logging for consistency. While the learning aboutself.stdout.write()applies to Command class methods, these helper classes don't have access toself.stdout.Based on learnings, consider using
logger.info()for progress messages, which provides better control over log levels and output destinations.🔎 Examples of recommended changes:
- print("Rendering slide image") + logger.info("Rendering slide image: %s", self.name)- print("Saved image to path:", self.image_output_path) + logger.info("Saved %s image to: %s", self.name, self.image_output_path)- print("Using pre-set transcript:", self.transcript) + logger.info("Using pre-set transcript for %s", self.name)- print("Generating transcript from prompt:", self.transcript_input) + logger.info("Generating transcript for %s", self.name)- print("Generated slide transcript:", self.transcript) + logger.info("Generated transcript for %s", self.name)- print("Generated audio at path:", self.audio_output_path) + logger.info("Generated audio for %s at: %s", self.name, self.audio_output_path)- print("Generated video for:", self.name) + logger.info("Generated video for: %s", self.name)Also applies to: 65-65, 83-83, 89-89, 107-107, 125-125, 146-146
154-154: Add missing type hint for snapshot parameter.The
snapshotparameter inSlideBuilder.__init__lacks a type hint. For consistency with the rest of the codebase and to improve IDE support, add theSnapshottype annotation.🔎 Apply this diff:
- def __init__(self, snapshot, output_dir: Path) -> None: + def __init__(self, snapshot: Snapshot, output_dir: Path) -> None: """Initialize SlideBuilder.""" self.snapshot = snapshot self.output_dir = output_dir
161-161: Replace print statements in Command.handle with self.stdout.write.Lines 161 and 238 use
print()for user-facing output in contexts where the Command instance is available.Based on learnings, Django management commands should use
self.stdout.write()instead ofprint()for user-facing output, as it follows Django conventions and improves testability.🔎 Apply these changes:
For line 161, pass the command's stdout writer to SlideBuilder and use it:
# In SlideBuilder.__init__, add a writer parameter def __init__(self, snapshot: Snapshot, output_dir: Path, writer=None) -> None: """Initialize SlideBuilder.""" self.snapshot = snapshot self.output_dir = output_dir self.writer = writer # In create_intro_slide def create_intro_slide(self) -> Slide: """Create an introduction slide.""" if self.writer: self.writer.write("Generating introduction slide for snapshot\n") # ... rest of methodFor line 238 in Command.handle:
- print("Generating video for snapshot:", snapshot_key) + self.stdout.write(f"Generating video for snapshot: {snapshot_key}\n")Then update the Command.handle call to SlideBuilder:
- slide_builder = SlideBuilder(snapshot, output_dir) + slide_builder = SlideBuilder(snapshot, output_dir, self.stdout)Also applies to: 238-238
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
backend/apps/common/eleven_labs.py (3)
ElevenLabs(17-137)set_text(43-55)generate_to_file(120-137)backend/apps/common/open_ai.py (4)
OpenAi(13-105)set_prompt(64-76)set_input(36-48)complete(78-105)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/apps/common/eleven_labs.py (1)
168-193: Validate text attribute before API call.The
generate()method accessesself.textwithout verifying it was set viaset_text(). Ifset_text()was never called, this raises anAttributeError.🔎 Add validation:
def generate(self) -> bytes | None: """Generate audio from text. Returns: bytes | None: The audio bytes or None on error. """ + if not hasattr(self, "text") or not self.text: + logger.error("Text not set. Call set_text() before generate().") + return None + try: audio_iterator = self.client.text_to_speech.convert( model_id=self.model_id, output_format=self.output_format, text=self.text, voice_id=self.voice_id, voice_settings={ "similarity_boost": self.similarity_boost, "stability": self.stability, "style": self.style, "use_speaker_boost": self.use_speaker_boost, }, ) return b"".join(audio_iterator) except Exception: logger.exception("An error occurred during ElevenLabs API request.") return Nonebackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
53-53: Replace print() with self.stdout.write() for command output.Multiple
print()statements are used throughout the command. In Django management commands, useself.stdout.write()for user-facing output to follow Django conventions and improve testability.Based on learnings, Django management commands should prefer
self.stdout.write()for stdout output.🔎 Example changes:
In the
Slidemethods, you'll need to pass the command instance or use a different approach since these are not methods ofCommand. Consider:Option 1: Add logging instead of print for Slide methods:
- print("Rendering slide image") + logger.info("Rendering slide image: %s", self.name)Option 2: Pass stdout writer to Slide methods as a parameter.
For Command.handle():
- print("Generating video for snapshot:", snapshot_key) + self.stdout.write(f"Generating video for snapshot: {snapshot_key}")For SlideBuilder methods, similar approach needed (pass stdout or use logging).
Also applies to: 65-65, 83-83, 89-89, 107-107, 125-125, 146-146, 161-161, 179-179, 293-293
🧹 Nitpick comments (4)
backend/apps/common/eleven_labs.py (1)
43-46: Validate API key before initializing client.The ElevenLabsClient is instantiated with
settings.ELEVENLABS_API_KEYwithout checking if the key exists or is non-empty. If misconfigured, API calls will fail later with unclear errors.🔎 Add validation:
+ api_key = settings.ELEVENLABS_API_KEY + if not api_key: + msg = "ELEVENLABS_API_KEY is not configured in settings" + raise ValueError(msg) + self.client = ElevenLabsClient( - api_key=settings.ELEVENLABS_API_KEY, + api_key=api_key, timeout=30, )backend/apps/owasp/templates/video/slides/projects.html (1)
8-38: Inconsistent project display between layouts.The template switches layouts at 15 projects but has inconsistencies:
- Dense layout (≤15): shows name, leaders, and created_at
- Sparse layout (>15): shows only name, capped at 25 projects via
slice:":25"This means 16-24 projects are shown with less information, and project 26+ are silently omitted without indication to the viewer.
Consider:
- Adding a count indicator when projects are truncated:
"Showing 25 of {{ project_count }} projects"- Making the data shown consistent across both layouts
- Documenting the 15/25 thresholds or extracting them as template context variables
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
238-250: Handle potential ffmpeg errors in video concatenation.The
_combine_videos()method calls ffmpeg operations without try-except blocks. If concatenation fails (e.g., due to incompatible formats or missing files), the exception will propagate uncaught.🔎 Add error handling:
def _combine_videos(self, output_path: Path) -> None: """Combine all slide videos into final video.""" + try: inputs = [ffmpeg.input(str(slide.video_output_path)) for slide in self.slides] video_streams = [inp.video for inp in inputs] audio_streams = [inp.audio for inp in inputs] joined_video = ffmpeg.concat(*video_streams, v=1, a=0) joined_audio = ffmpeg.concat(*audio_streams, v=0, a=1) ffmpeg.output(joined_video, joined_audio, str(output_path)).overwrite_output().run( capture_stdout=True, capture_stderr=True ) + except ffmpeg.Error: + logger.exception("Error combining videos into final output") + raise
256-274: Add type hint for parser parameter.The
add_argumentsmethod parameterparserlacks a type hint, reducing IDE support and type safety.🔎 Add type hint:
+ from argparse import ArgumentParser + - def add_arguments(self, parser) -> None: + def add_arguments(self, parser: ArgumentParser) -> None: """Add command-line arguments to the parser. Args: - parser (argparse.ArgumentParser): The argument parser instance. + parser: The argument parser instance. """
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/apps/common/eleven_labs.py(1 hunks)backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)backend/apps/owasp/templates/video/base.html(1 hunks)backend/apps/owasp/templates/video/slides/projects.html(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
backend/apps/common/eleven_labs.py (3)
ElevenLabs(17-212)set_text(126-138)generate_to_file(195-212)backend/apps/common/open_ai.py (4)
OpenAi(13-105)set_prompt(64-76)set_input(36-48)complete(78-105)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
backend/apps/owasp/templates/video/base.html (1)
1-139: Base template structure looks good.The fixed viewport (1920×1080), inline utility CSS, and Arial font choice are appropriate for WeasyPrint-based video slide rendering. The template provides a solid foundation for the video generation pipeline.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
201-208: Add guard against empty projects list.Lines 204-207 format project names but don't handle the case when
project_namesis empty. If a snapshot has no new projects, accessingproject_names[-1]will raise anIndexError.In a previous review thread, you mentioned skipping the projects section when there are no new projects. However, the current code still calls
create_projects_slide()unconditionally (line 308). You should either:
- Add a guard in
Command.handle()to skip creating the projects slide whensnapshot.new_projects.count() == 0, or- Add validation here to handle the empty case gracefully.
🔎 Option 1: Skip projects slide in Command.handle() (recommended)
# In Command.handle() around line 308 generator.append_slide(slide_builder.create_intro_slide()) # Only add projects slide if there are new projects if snapshot.new_projects.exists(): generator.append_slide(slide_builder.create_projects_slide())🔎 Option 2: Guard in create_projects_slide()
project_names = [ p.name.replace("OWASP ", "") for p in new_projects[:MAX_TRANSCRIPT_PROJECTS] ] + if not project_names: + raise ValueError("Cannot create projects slide: no projects available") + formatted_project_names = ( f"{', '.join(project_names[:-1])}, and {project_names[-1]}" if len(project_names) > 1 else project_names[0] )Based on learnings, prefer Option 1 as it aligns with your stated intention to skip the section entirely when there are no projects.
🧹 Nitpick comments (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
108-115: Consider raising exception immediately when transcript generation fails.When
open_ai.complete()returnsNone, the method logs an error and returns early, leavingself.transcriptasNone. Later,generate_audio()will detect this and raise aRuntimeError. This deferred error handling makes debugging harder because the error message won't indicate that transcript generation failed.For consistency with
generate_audio()and clearer error reporting, consider raising immediately:🔎 Proposed refactor for immediate error raising
transcript = open_ai.complete() if not transcript: logger.error("Error generating transcript for %s", self.name) - return + raise RuntimeError(f"Failed to generate transcript for slide: {self.name}") self.transcript = transcript print("Generated slide transcript:", self.transcript)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py(1 hunks)backend/apps/owasp/templates/video/base.html(1 hunks)backend/apps/owasp/templates/video/slides/projects.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/templates/video/base.html
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (4)
backend/apps/common/eleven_labs.py (3)
ElevenLabs(17-212)set_text(126-138)generate_to_file(195-212)backend/apps/common/open_ai.py (4)
OpenAi(13-105)set_prompt(64-76)set_input(36-48)complete(78-105)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)backend/apps/owasp/api/internal/nodes/snapshot.py (1)
new_projects(45-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (6)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (6)
30-52: LGTM!The dataclass is well-structured with proper type hints and validation logic. The
__post_init__method correctly enforces that exactly one oftranscriptortranscript_inputmust be provided, preventing invalid states.
53-81: LGTM!The resource cleanup pattern is well-implemented. Initializing
pagetoNonebefore the try block and using guarded cleanup in the finally block ensures resources are properly released even if errors occur during rendering.
116-136: LGTM!The error handling is robust. The method validates that a transcript exists before attempting audio generation and raises meaningful exceptions when operations fail, ensuring errors are caught early with clear context.
137-158: LGTM!The FFmpeg integration properly handles errors by catching
ffmpeg.Error, logging with context, and re-raising to ensure failures aren't silently swallowed. The video generation parameters (aac audio codec, libx264 video codec, yuv420p pixel format) are appropriate for broad compatibility.
226-260: LGTM!The Generator orchestrates the video pipeline effectively. The sequence (render → transcript → audio → video → combine) is logical, and the FFmpeg concatenation properly handles both video and audio streams separately before merging them into the final output.
285-310: LGTM with one note.The command structure follows Django conventions properly. The environment variable setup is correctly placed in
handle()rather than at module level, and the snapshot fetching includes appropriate error handling.Note: Line 308 unconditionally creates the projects slide. See the comment on lines 201-208 regarding handling empty projects lists.
rudransh-shrivastava
force-pushed
the
feature/community-snapshot-videos
branch
from
3a54683 to
9c15eda
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
backend/apps/owasp/templates/video/slides/projects.html (1)
1-22: LGTM!The template structure is clean and follows Django best practices. The previously flagged
w-76class is now properly defined inbase.html(lines 154-156), resolving the earlier concern.backend/apps/owasp/templates/video/base.html (1)
1-172: LGTM!The base template is well-structured for video rendering with WeasyPrint. All utility classes are properly defined inline, including the custom
w-76class. The use of Arial font is appropriate for PDF/image rendering.backend/apps/common/eleven_labs.py (2)
43-46: Validate API key before initialization.The code accesses
settings.ELEVENLABS_API_KEYwithout checking if it's configured. If the key is missing or empty, the ElevenLabsClient initialization may fail with an unclear error message.🔎 Add validation before client initialization
+ api_key = settings.ELEVENLABS_API_KEY + if not api_key: + msg = "ELEVENLABS_API_KEY is not configured in settings" + raise ValueError(msg) + self.client = ElevenLabsClient( - api_key=settings.ELEVENLABS_API_KEY, + api_key=api_key, timeout=30, )
168-193: Validate text is set before generating audio.The
generate()method accessesself.textwithout checking ifset_text()was called first. If the text attribute doesn't exist, this will raise anAttributeError.🔎 Add validation before API call
def generate(self) -> bytes | None: """Generate audio from text. Returns: bytes | None: The audio bytes or None on error. """ + if not hasattr(self, "text") or not self.text: + logger.error("Text not set. Call set_text() before generate().") + return None + try: audio_iterator = self.client.text_to_speech.convert( model_id=self.model_id, output_format=self.output_format, text=self.text, voice_id=self.voice_id, voice_settings={ "similarity_boost": self.similarity_boost, "stability": self.stability, "style": self.style, "use_speaker_boost": self.use_speaker_boost, }, ) return b"".join(audio_iterator) except Exception: logger.exception("An error occurred during ElevenLabs API request.") return Nonebackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
201-208: Handle the case when no new projects exist.Lines 204-208 will raise an
IndexErrorifproject_namesis empty (when there are no new projects or fewer thanMAX_TRANSCRIPT_PROJECTS). While you mentioned this is "quite rare," it's better to handle gracefully to avoid runtime failures.🔎 Add validation for empty project list
project_names = [ p.name.replace("OWASP ", "") for p in new_projects[:MAX_TRANSCRIPT_PROJECTS] ] + if not project_names: + # Skip or handle the no-projects case + logger.warning("No projects available for transcript generation") + formatted_project_names = "" + elif len(project_names) > 1: + formatted_project_names = f"{', '.join(project_names[:-1])}, and {project_names[-1]}" + else: + formatted_project_names = project_names[0] - formatted_project_names = ( - f"{', '.join(project_names[:-1])}, and {project_names[-1]}" - if len(project_names) > 1 - else project_names[0] - )Alternatively, you could skip creating the projects slide entirely in
Command.handle()whensnapshot.new_projects.exists()returnsFalse, which would be cleaner.
🧹 Nitpick comments (2)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
60-60: Consider usingself.stdout.write()for command output.The
print()statements could be replaced withself.stdout.write()to align with Django management command conventions. However, since this is WIP and you've indicated these will be addressed later, this is just a reminder.Based on learnings, Django commands should use
self.stdout.write()for user-facing output instead ofprint().Also applies to: 72-72
307-308: Consider checking if new projects exist before creating the slide.To avoid the potential
IndexErrorincreate_projects_slide()when there are no new projects, you could add a conditional check here:generator.append_slide(slide_builder.create_intro_slide()) - generator.append_slide(slide_builder.create_projects_slide()) + if snapshot.new_projects.exists(): + generator.append_slide(slide_builder.create_projects_slide()) + else: + logger.info("Skipping projects slide - no new projects in snapshot") generator.generate_video(output_dir / f"{snapshot_key}_snapshot.mp4")This pairs with the earlier suggestion to handle empty project lists and provides a cleaner solution.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
backend/.env.examplebackend/apps/common/eleven_labs.pybackend/apps/owasp/Makefilebackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.pybackend/apps/owasp/templates/video/base.htmlbackend/apps/owasp/templates/video/slides/intro.htmlbackend/apps/owasp/templates/video/slides/projects.htmlbackend/docker/Dockerfile.localbackend/pyproject.tomlbackend/settings/base.pycspell/custom-dict.txt
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/docker/Dockerfile.local
- backend/.env.example
- backend/settings/base.py
- backend/apps/owasp/templates/video/slides/intro.html
- backend/pyproject.toml
- cspell/custom-dict.txt
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
backend/apps/owasp/Makefile (1)
25-27: LGTM!The new Makefile target follows the established pattern and correctly invokes the Django management command with the required
--snapshot-keyparameter.backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (4)
30-52: LGTM!The
Slidedataclass is well-structured with proper validation in__post_init__to ensure exactly one oftranscriptortranscript_inputis provided. Type hints are clear and complete.
82-114: LGTM!The transcript generation logic properly handles the case where
open_ai.complete()returnsNone, with appropriate error logging and early return. The validation logic for pre-set transcripts and transcript input is also correct.
116-157: LGTM!Both
generate_audio()andgenerate_video()methods have proper validation and error handling. The audio generation checks for transcript existence before proceeding, and video generation properly re-raises exceptions after logging. FFmpeg parameters are appropriate for video encoding.
226-259: LGTM!The
Generatorclass orchestrates the video generation pipeline correctly. The slide processing loop and video concatenation logic using ffmpeg are well-structured. Type hints are properly included.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
212-212: Remove or utilize unused OpenAi instance.
self.open_aiis instantiated but never used anywhere in theGeneratorclass. Since transcripts are currently hardcoded inSlideBuildermethods, this instance serves no purpose and should either be removed to reduce unnecessary dependencies, or integrated if dynamic transcript generation is planned.🔎 If not needed, remove it
class Generator: def __init__(self) -> None: """Initialize Video Generator.""" self.eleven_labs = ElevenLabs(stability=ELEVENLABS_STABILITY, style=ELEVENLABS_STYLE) - self.open_ai = OpenAi() self.slides: list[Slide] = []
285-285: Clarify output directory behavior and consider cleanup of intermediate files.Line 285 uses
OUTPUT_DIR(global temp directory) for slide intermediate files, while the user-providedoutput_diris only used for the final merged video (line 295). This means:
- User's
--output-diraffects only the final.mp4location- Intermediate files (images, audio, per-slide videos) remain in the system temp directory indefinitely
While using temp for intermediates is reasonable to avoid cluttering the user's directory, these files are never cleaned up after
generate_video()completes. Consider either documenting this behavior or implementing cleanup of intermediate files after the merge succeeds.🔎 Optional: Clean up intermediate files after merge
def handle(self, *args, **options) -> None: """Handle the command execution.""" # Required to cache fonts os.environ["XDG_CACHE_HOME"] = str(Path(tempfile.gettempdir()) / "cache") snapshot_key = options["snapshot_key"] output_dir = options["output_dir"] output_dir.mkdir(parents=True, exist_ok=True) + + # Create unique temp directory for intermediate files + import tempfile + temp_dir = Path(tempfile.mkdtemp(prefix=f"snapshot_{snapshot_key}_")) snapshot = Snapshot.objects.filter( key__iexact=snapshot_key, status=Snapshot.Status.COMPLETED ).first() if not snapshot: logger.error("No completed snapshot found for key: %s", snapshot_key) return print("Generating video for snapshot:", snapshot_key) - slide_builder = SlideBuilder(snapshot, OUTPUT_DIR) + slide_builder = SlideBuilder(snapshot, temp_dir) generator = Generator() generator.append_slide(slide_builder.create_intro_slide()) generator.append_slide(slide_builder.create_sponsors_slide()) if snapshot.new_projects.exists(): generator.append_slide(slide_builder.create_projects_slide()) else: print("Skipping projects slide - No new projects in snapshot.") generator.generate_video(output_dir / f"{snapshot_key}_snapshot.mp4") + + # Cleanup intermediate files + import shutil + shutil.rmtree(temp_dir, ignore_errors=True)This creates a unique temporary directory per run (avoiding conflicts) and cleans it up after the final video is generated.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (1)
244-244: Add type hint for the parser parameter.The
parserparameter lacks a type annotation. Adding it improves type safety and IDE support.🔎 Proposed improvement
+from argparse import ArgumentParser + # ... existing imports ... - def add_arguments(self, parser) -> None: + def add_arguments(self, parser: ArgumentParser) -> None: """Add command-line arguments to the parser.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (1)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
backend/apps/common/eleven_labs.py (3)
ElevenLabs(17-212)set_text(126-138)generate_to_file(195-212)backend/apps/owasp/api/internal/queries/snapshot.py (1)
snapshot(14-22)backend/apps/owasp/api/internal/queries/sponsor.py (1)
sponsors(14-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Outdated
Show resolved
Hide resolved
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Great work! Some comments to address:
backend/apps/common/eleven_labs.py
Outdated
|
|
||
| return None | ||
|
|
||
| def generate_to_file(self, file_path: Path) -> bool: |
There was a problem hiding this comment.
Let's separate concerns here
| def generate_to_file(self, file_path: Path) -> bool: | |
| def save(self, contents: bytes, file_path: Path): |
backend/apps/common/eleven_labs.py
Outdated
| ) | ||
|
|
||
| return b"".join(audio_iterator) | ||
| except Exception: |
There was a problem hiding this comment.
Let's allow it fail if no content was generated? Normally you don't need that wide exception handling at all.
backend/apps/owasp/video.py
Outdated
| self.image_path.parent.mkdir(parents=True, exist_ok=True) | ||
| pil_image.save(self.image_path, IMAGE_FORMAT) | ||
|
|
||
| except Exception: |
There was a problem hiding this comment.
Same here -- can we avoid wide exception catching?
backend/apps/owasp/video.py
Outdated
| return cleaned[0] | ||
| return "" | ||
|
|
||
| def create_intro_slide(self) -> Slide: |
There was a problem hiding this comment.
Let's use add_ prefix for slides adding.
backend/apps/owasp/video.py
Outdated
| return Slide( | ||
| context={ | ||
| "sponsors": sponsors, | ||
| "title": "Our Sponsors", |
There was a problem hiding this comment.
| "title": "Our Sponsors", | |
| "title": "OWASP Foundation Sponsors", |
backend/apps/owasp/video.py
Outdated
| return Slide( | ||
| context={ | ||
| "projects": projects_data, | ||
| "title": f"New Projects ({project_count})", |
There was a problem hiding this comment.
Put the count first and pluralize titles for all entities.
backend/apps/owasp/video.py
Outdated
|
|
||
| chapters = [ | ||
| { | ||
| "location": f"{c.region}, {c.country}" if c.region else c.country, |
There was a problem hiding this comment.
Use suggested_location here.
backend/apps/owasp/video.py
Outdated
| output_dir=self.output_dir, | ||
| name="thank_you", | ||
| template_name="video/slides/thank_you.html", | ||
| transcript="Thanks for tuning in!... Visit Nest at Nest dot O-WASP dot org " |
There was a problem hiding this comment.
| transcript="Thanks for tuning in!... Visit Nest at Nest dot O-WASP dot org " | |
| transcript="Thanks for tuning in!... Visit Nest dot O-WASP dot org " |
backend/pyproject.toml
Outdated
| strawberry-graphql-django = "^0.72.0" | ||
| thefuzz = "^0.22.1" | ||
| pyparsing = "^3.2.3" | ||
| weasyprint = "^67.0" |
There was a problem hiding this comment.
Let's make it right from the start -- I don't think we want to add these deps into the backend image. Let's create a new image just for the snapshot video generation based on the backend image.
There was a problem hiding this comment.
Yep, added it.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.py (1)
76-119: Remove redundant import.Line 113 imports
Pathfrompathlib, but this is already imported at line 4 of the file. This redundant import should be removed.🔎 Proposed fix
assert mock_generator_instance.append_slide.call_count == 4 - from pathlib import Path mock_generator_instance.generate_video.assert_called_once()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/apps/common/eleven_labs.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.pybackend/apps/owasp/video.pybackend/tests/apps/common/eleven_labs_test.pybackend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/tests/apps/owasp/video_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/apps/common/eleven_labs.py
- backend/tests/apps/common/eleven_labs_test.py
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:296-296
Timestamp: 2025-12-28T15:20:59.650Z
Learning: In `backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py`, the SlideBuilder is intentionally initialized with the module constant OUTPUT_DIR (temp directory) rather than the user-specified output_dir flag. This is by design: intermediate files (images, audio, per-slide videos) are generated in temp, while the final merged video is saved to the user-specified output directory.
📚 Learning: 2025-12-28T15:20:59.650Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:296-296
Timestamp: 2025-12-28T15:20:59.650Z
Learning: In `backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py`, the SlideBuilder is intentionally initialized with the module constant OUTPUT_DIR (temp directory) rather than the user-specified output_dir flag. This is by design: intermediate files (images, audio, per-slide videos) are generated in temp, while the final merged video is saved to the user-specified output directory.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.pybackend/tests/apps/owasp/video_test.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.pybackend/tests/apps/owasp/video_test.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.pybackend/tests/apps/owasp/video_test.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-06-18T20:00:23.899Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1634
File: frontend/src/app/api/auth/[...nextauth]/route.ts:30-55
Timestamp: 2025-06-18T20:00:23.899Z
Learning: The OWASP Nest application has logging disabled, so avoid suggesting console.log, console.error, or any other logging statements in code review suggestions.
Applied to files:
backend/apps/owasp/video.py
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (2)
backend/apps/owasp/video.py (2)
backend/apps/common/eleven_labs.py (4)
ElevenLabs(18-218)save(209-218)set_text(144-156)generate(186-207)backend/apps/owasp/models/sponsor.py (1)
SponsorType(21-27)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
backend/apps/owasp/api/internal/queries/snapshot.py (2)
snapshot(14-22)snapshots(25-31)backend/apps/owasp/video.py (11)
SlideBuilder(125-325)VideoGenerator(328-397)add_intro_slide(158-174)add_sponsors_slide(176-202)add_projects_slide(204-239)add_chapters_slide(241-271)add_releases_slide(273-314)add_thank_you_slide(316-325)append_slide(336-343)generate_video(345-364)cleanup(392-397)backend/tests/apps/owasp/video_test.py (3)
slide_builder(133-135)generator(316-319)slide(17-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
🔇 Additional comments (22)
backend/tests/apps/owasp/video_test.py (6)
1-13: LGTM - Imports and module setup.The imports are appropriate for the test suite. The module correctly imports the public API surface from
apps.owasp.videoincluding constants and classes under test.
15-68: LGTM - TestSlide fixture and property/render tests.The fixture provides a well-structured sample
Slideinstance. The property tests correctly verify path construction using the module constants. Therender_and_save_image_successtest properly mocks the PDF rendering pipeline and verifies resource cleanup calls.
70-117: LGTM - Audio and video generation tests.Good coverage for both success and failure paths. The exception tests correctly import the real
ffmpeg.Errorto verify exception propagation, and the logger mock properly asserts that exceptions are logged.
120-154: LGTM - SlideBuilder fixtures and format_names_for_transcript tests.Parameterized tests effectively cover edge cases: single name, two names, three names, more than limit (truncated to 3), and empty list. This aligns with the
TRANSCRIPT_NAMES_LIMIT = 3constant.
156-311: LGTM - Slide creation method tests.The tests properly mock ORM objects and verify slide properties. The tests for
add_projects_slide,add_chapters_slide, andadd_releases_slidecorrectly validate both theNonereturn case (no data) and the successful slide creation case.
314-391: LGTM - VideoGenerator tests.The tests properly mock the ElevenLabs client during initialization and verify the slide lifecycle: appending slides, generating per-slide assets, and merging. The exception handling test for
merge_videoscorrectly verifies logging behavior.backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
1-14: LGTM - Module setup and imports.Clean module docstring, appropriate imports, and well-structured logger setup. The
DOCKER_CONTAINER_NAMEconstant provides clear guidance for users.
17-31: LGTM - Command class and argument configuration.The help text is clear and the argument definition properly documents the expected format with examples.
33-72: LGTM - Command handle method implementation.The implementation follows Django management command best practices:
- Uses
self.stdout.write()for user-facing output- Uses
self.style.SUCCESS()for styled success message- Properly filters
Noneslides before appending- Calls
cleanup()to remove intermediate files- Provides helpful docker cp guidance
One minor note: Line 41 uses
if not snapshots:for the empty check. Based on learnings, while this works correctly (QuerySets support boolean evaluation), usingsnapshots.exists()would be slightly more efficient as it avoids fetching results. This is acceptable given the context.backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.py (4)
1-23: LGTM - Test setup and fixtures.The
sys.modules["weasyprint"] = MagicMock()on line 10 is correctly placed before the command import to avoid WeasyPrint initialization during tests. The command fixture properly mocks stdout for output verification.
25-42: LGTM - Command interface tests.Good coverage of the command's public interface: help text, inheritance from BaseCommand, and argument configuration.
44-75: LGTM - No snapshots handling test.The test correctly verifies that when no completed snapshots are found, the command logs an error and does not instantiate SlideBuilder or VideoGenerator.
121-178: LGTM - Additional handle method tests.Good coverage for:
- Filtering out
Noneslides (verifies only 2 slides appended when 4 returnNone)- Docker cp command output verification
The tests effectively validate the command's behavior with various slide configurations.
backend/apps/owasp/video.py (9)
1-38: LGTM - Module setup and constants.The
XDG_CACHE_HOMEenvironment variable (line 12) is correctly set before the WeasyPrint import to enable font caching. The constants are well-organized and appropriately named.Note: Setting
os.environ["XDG_CACHE_HOME"]at module level affects the global environment when this module is imported. This is documented as required for WeasyPrint and is acceptable given the module's purpose.
41-65: LGTM - Slide dataclass and path properties.Clean dataclass design with appropriate fields. The path properties correctly use the module constants for file extensions.
66-87: LGTM - render_and_save_image implementation.Proper resource management using try/finally to ensure PDF document and page are closed even if an exception occurs. The pre-initialization of
pageandNone(lines 68-69) ensures the finally block handles partial initialization correctly.
88-123: LGTM - Audio and video generation methods.The
generate_and_save_audiomethod has a clean interface delegating to the ElevenLabs client. Thegenerate_and_save_videomethod properly handles ffmpeg errors with logging and re-raises for caller handling.
125-157: LGTM - SlideBuilder initialization and format_names_for_transcript.The static method
format_names_for_transcripthandles all edge cases correctly:
- Empty list returns empty string
- Single name returns the name
- Multiple names use proper comma-separation with "and" before the last
The docstrings are comprehensive with proper Args and Returns documentation.
158-202: LGTM - Intro and sponsors slide creation.The
add_intro_slidecorrectly handles both single and multi-snapshot scenarios for naming. Theadd_sponsors_slidefollows the established codebase pattern for sponsor sorting (matching the GraphQL query inbackend/apps/owasp/api/internal/queries/sponsor.py). Based on learnings, this in-memory sorting approach is intentional.
204-271: LGTM - Projects and chapters slide creation.Both methods correctly:
- Use
.exists()for efficient empty check (per learnings)- Return
Nonewhen no data exists- Format names using the shared helper method
- Include appropriate context for template rendering
273-325: LGTM - Releases and thank you slide creation.The
add_releases_slidemethod includes a defensive check for emptytop_projects(lines 288-289), returningNoneto skip the slide gracefully. This addresses the edge case discussed in past reviews. Theadd_thank_you_slideprovides a simple closing slide.
328-397: LGTM - VideoGenerator class.Well-structured orchestration class:
- Proper initialization with ElevenLabs client and slides list
generate_videocoordinates the full pipeline (render → audio → video → merge)merge_videoshas proper error handling with loggingcleanupsafely removes intermediate files, checking existence before unlinkingThe docstrings are comprehensive with proper Args, Returns, and Raises documentation.
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
backend/apps/common/eleven_labs.py (2)
193-214: Consider adding a check forself.textbefore API call.The
generate()method assumesset_text()was called. If not, anAttributeErrorwill occur. While the current usage pattern is safe (based on the codebase review), a defensive check would make the API more robust.🔎 Optional: Add defensive validation
def generate(self) -> bytes: """Generate audio from text. Returns: bytes: The audio bytes. """ + if not hasattr(self, "text") or not self.text: + msg = "Text not set. Call set_text() before generate()." + raise ValueError(msg) + audio_iterator = self.client.text_to_speech.convert( model_id=self.model_id, output_format=self.output_format, text=self.text, voice_id=self.voice_id, voice_settings=VoiceSettings( similarity_boost=self.similarity_boost, speed=self.speed, stability=self.stability, style=self.style, use_speaker_boost=self.use_speaker_boost, ), ) return b"".join(audio_iterator)
21-66: Consider validating parameter ranges in constructor.The docstrings specify valid ranges (e.g., similarity_boost: 0.0-1.0, speed: 0.25-4.0), but there's no runtime validation. Invalid values will be caught by the ElevenLabs API, but early validation would provide clearer error messages.
🔎 Example validation for numeric parameters
def __init__( self, model_id: str = "eleven_multilingual_v2", output_format: str = "mp3_44100_128", similarity_boost: float = 0.75, speed: float = 1.0, stability: float = 0.5, style: float = 0.0, voice_id: str = "1SM7GgM6IMuvQlz2BwM3", # cspell:disable-line *, use_speaker_boost: bool = True, ) -> None: """ElevenLabs constructor. Args: model_id (str): The model to use. output_format (str): Audio output format. similarity_boost (float): Voice consistency (0.0-1.0). speed (float): Speech speed (0.25-4.0, default 1.0). stability (float): Voice stability (0.0-1.0). style (float): Style exaggeration (0.0-1.0). use_speaker_boost (bool): Enable speaker clarity boost. voice_id (str): The voice ID to use. Raises: ValueError: If the ElevenLabs API key is not set. """ if not (api_key := os.getenv("DJANGO_ELEVENLABS_API_KEY")): error_msg = "DJANGO_ELEVENLABS_API_KEY environment variable not set" raise ValueError(error_msg) + + if not 0.0 <= similarity_boost <= 1.0: + msg = f"similarity_boost must be between 0.0 and 1.0, got {similarity_boost}" + raise ValueError(msg) + if not 0.25 <= speed <= 4.0: + msg = f"speed must be between 0.25 and 4.0, got {speed}" + raise ValueError(msg) + if not 0.0 <= stability <= 1.0: + msg = f"stability must be between 0.0 and 1.0, got {stability}" + raise ValueError(msg) + if not 0.0 <= style <= 1.0: + msg = f"style must be between 0.0 and 1.0, got {style}" + raise ValueError(msg) self.client = ElevenLabsClient( api_key=api_key, timeout=30, )backend/docker/Dockerfile.video (1)
68-68: Container has no default command.The Dockerfile ends without a
CMDorENTRYPOINT. While the Makefile provides an explicit command, this means:
- Running the container without arguments will fail
- No self-documenting default behavior
- Less convenient for manual testing
🔎 Consider adding a helpful default command
USER owasp + +CMD ["python", "manage.py", "help", "owasp_generate_community_snapshot_video"]This would display help text when run without arguments, making the container more self-documenting.
backend/apps/owasp/Makefile (1)
25-34: Add validation for requiredsnapshot_keyparameter.The target uses
$(snapshot_key)without validation. If not provided, the container will run with an incomplete command, causing a late failure with unclear errors.🔎 Add pre-flight validation
owasp-generate-community-snapshot-video: + @if [ -z "$(snapshot_key)" ]; then \ + echo "Error: snapshot_key parameter is required"; \ + echo "Usage: make owasp-generate-community-snapshot-video snapshot_key=<key>"; \ + exit 1; \ + fi @docker build -f backend/docker/Dockerfile.video backend/ \ -t nest-video-backend @docker run \ --mount type=bind,src="$(PWD)",dst=/home/owasp/generated_videos \ --env-file backend/.env \ --network nest-local_nest-network \ --rm nest-video-backend \ python manage.py owasp_generate_community_snapshot_video $(snapshot_key) /home/owasp/generated_videos
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.gitignorebackend/apps/common/eleven_labs.pybackend/apps/owasp/Makefilebackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.pybackend/apps/owasp/video.pybackend/docker/Dockerfilebackend/docker/Dockerfile.localbackend/docker/Dockerfile.videobackend/pyproject.tomlbackend/tests/apps/common/eleven_labs_test.pybackend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.py
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/tests/apps/common/eleven_labs_test.py
- backend/pyproject.toml
- backend/docker/Dockerfile.local
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:296-296
Timestamp: 2025-12-28T15:20:59.650Z
Learning: In `backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py`, the SlideBuilder is intentionally initialized with the module constant OUTPUT_DIR (temp directory) rather than the user-specified output_dir flag. This is by design: intermediate files (images, audio, per-slide videos) are generated in temp, while the final merged video is saved to the user-specified output directory.
📚 Learning: 2025-12-28T15:20:59.650Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:296-296
Timestamp: 2025-12-28T15:20:59.650Z
Learning: In `backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py`, the SlideBuilder is intentionally initialized with the module constant OUTPUT_DIR (temp directory) rather than the user-specified output_dir flag. This is by design: intermediate files (images, audio, per-slide videos) are generated in temp, while the final merged video is saved to the user-specified output directory.
Applied to files:
backend/apps/owasp/Makefilebackend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/common/eleven_labs.pybackend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.pybackend/apps/common/eleven_labs.pybackend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-08-19T15:37:57.180Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/settings/base.py:34-37
Timestamp: 2025-08-19T15:37:57.180Z
Learning: In django-configurations, IS_GOOGLE_AUTH_ENABLED computed from Value/SecretValue descriptors works correctly at runtime despite descriptors being objects at class definition time, as evidenced by extensive test coverage using override_settings that successfully validates both enabled/disabled states.
Applied to files:
backend/apps/common/eleven_labs.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/video.pybackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-06-18T20:00:23.899Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1634
File: frontend/src/app/api/auth/[...nextauth]/route.ts:30-55
Timestamp: 2025-06-18T20:00:23.899Z
Learning: The OWASP Nest application has logging disabled, so avoid suggesting console.log, console.error, or any other logging statements in code review suggestions.
Applied to files:
backend/apps/owasp/video.py
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🧬 Code graph analysis (3)
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.py (2)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (3)
Command(17-78)add_arguments(20-36)handle(38-78)backend/apps/owasp/video.py (9)
add_intro_slide(153-169)add_sponsors_slide(171-197)add_projects_slide(199-234)add_chapters_slide(236-266)add_releases_slide(268-309)add_thank_you_slide(311-320)append_slide(331-338)generate_video(340-359)cleanup(387-392)
backend/apps/owasp/video.py (2)
backend/apps/common/eleven_labs.py (4)
ElevenLabs(18-225)save(216-225)set_text(151-163)generate(193-214)backend/apps/owasp/models/sponsor.py (1)
SponsorType(21-27)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
backend/apps/owasp/api/internal/queries/snapshot.py (2)
snapshot(14-22)snapshots(25-31)backend/apps/owasp/video.py (6)
SlideBuilder(120-320)VideoGenerator(323-392)append_slide(331-338)video_path(57-59)generate_video(340-359)cleanup(387-392)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run Code Scan
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (25)
.gitignore (1)
31-31: LGTM!The ignore pattern correctly targets generated video output directories and is appropriately placed after related build artifacts.
backend/apps/common/eleven_labs.py (1)
49-51: LGTM! Environment variable validation is now explicit.The constructor properly validates the API key at initialization using
os.getenvand raises a clear error message, ensuring fail-fast behavior. This addresses the earlier concern about configuration validation.backend/docker/Dockerfile (1)
36-36: LGTM! Video dependencies appropriately excluded.The
--without videoflag correctly excludes video-specific dependencies from the main image, reducing size and maintaining clean separation with the dedicated video Dockerfile.backend/docker/Dockerfile.video (1)
9-9: Different APK cache directory name from main Dockerfile.Line 9 uses
/home/owasp/.cache/apk-backend-videowhile the main Dockerfile uses/home/owasp/.cache/apk. This difference may be intentional for cache isolation, but it means Docker cache layers won't be shared between the two builds.Is the different cache directory name intentional? If cache isolation isn't needed, using the same path would improve build times through shared cache layers.
backend/tests/apps/owasp/management/commands/owasp_generate_community_snapshot_video_test.py (4)
1-23: LGTM!The module-level weasyprint mock and fixture setup are correctly implemented. Mocking
weasyprintbefore importing the command avoids system dependency issues during test execution, and thenoqa: E402is appropriately used.
25-42: LGTM!The command surface tests appropriately verify help text, inheritance, and argument parser configuration. The assertion on
call_countis sufficient for verifying that both required arguments are registered.
44-77: LGTM!The test correctly verifies early-exit behavior when no completed snapshots are found. The class-level patches are efficiently applied, and the mock assertions confirm that the command doesn't proceed with slide generation when the snapshot query returns empty.
78-183: LGTM!The tests comprehensively cover the handle method's behavior:
- Valid snapshots with mixed None/non-None slides
- None-slide filtering logic
- Success message output with correct path information
The assertions verify that only non-None slides are appended to the generator and that the final video path is correctly constructed.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (5)
1-16: LGTM!Imports are clean and well-organized. The logger and constant are appropriately defined for the command's needs.
17-37: LGTM!The command interface is well-defined with clear help text and properly documented arguments. The examples in the help strings guide users on expected input formats.
38-49: LGTM!The snapshot query correctly filters by key prefix and completion status. The early-return pattern prevents unnecessary processing when no snapshots exist. The boolean evaluation of the QuerySet is valid and triggers the appropriate database query.
51-60: LGTM!The directory management correctly separates temporary intermediate files (images, audio, per-slide videos) from the final output. The
SlideBuilderinitialization withtemp_output_diris intentional per the established design pattern.Based on learnings, this separation ensures intermediate files are isolated in temp while the final merged video goes to the user-specified output directory.
62-78: LGTM!The slide generation workflow is well-structured:
- Slides are created in logical order
- None slides are filtered before being appended to the generator
- The final video is correctly saved to
video_output_dir(user-specified location)- Cleanup removes intermediate files after generation
- Success message uses styled output following Django conventions
backend/apps/owasp/video.py (12)
1-34: LGTM!Imports and constants are well-organized. The configuration values (speed, limits, formats) are appropriately defined as module-level constants for easy adjustment.
36-82: LGTM!The
Slidedataclass is well-structured with clean property accessors. Therender_and_save_imagemethod properly handles resource management by initializingpageandNonebefore the try block and conditionally closing them in the finally block, preventing potential NameError exceptions.
83-118: LGTM!Audio and video generation methods are well-implemented. The error handling in
generate_and_save_videoproperly logs failures with context and re-raises exceptions to propagate errors to callers.
120-152: LGTM!The
SlideBuilderinitialization andformat_names_for_transcripthelper are well-implemented. The formatting logic gracefully handles empty lists, single names, and multiple names with proper punctuation.
153-169: LGTM!The intro slide creation correctly handles both single-month and multi-month (year) snapshots. The command's early-exit ensures this method is never called with empty snapshots.
171-197: LGTM!The sponsors slide correctly follows the established pattern of including all sponsors (including
NOT_SPONSOR) and sorting them in-memory by tier priority. This aligns with the GraphQL sponsor query implementation.Based on learnings, this in-code sorting approach is the standard for sponsor lists in the OWASP backend.
199-234: LGTM!The projects slide creation properly handles the no-projects case by returning
None. The query and data transformation are well-structured, and the transcript formatting is clear.
236-266: LGTM!The chapters slide follows the same pattern as projects, correctly handling empty data by returning
None. The use ofsuggested_locationprovides useful geographic context.
268-309: LGTM!The releases slide correctly handles empty data cases at both the releases level and the top-projects level. The N+1 query pattern (lines 274-280) is acceptable for this local command context as previously discussed.
The bar width calculation and transcript formatting are well-implemented.
311-320: LGTM!The thank you slide is appropriately simple, with an empty context and a friendly closing transcript.
323-359: LGTM!The
VideoGeneratorclass is well-structured:
- Initializes ElevenLabs client with appropriate speed setting
append_slidemethod is straightforwardgenerate_videoorchestrates the full pipeline: render images, generate audio, create per-slide videos, merge, and return the final path
361-392: LGTM!The
merge_videosandcleanupmethods complete the video generation pipeline:
merge_videosproperly concatenates all slide videos and audio streams with appropriate error handling- The ffmpeg error logging and re-raising ensures failures are visible
cleanupefficiently removes all intermediate files after the final video is produced
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py (2)
47-49: Consider using.exists()for more efficient existence check.The boolean evaluation
if not snapshots:is functionally correct but triggers a full database query. Usingsnapshots.exists()would run a more efficientEXISTSquery without fetching objects.Based on learnings, while the current approach works,
.exists()is preferred for performance when you only need to know whether any rows exist.🔎 Suggested improvement
- if not snapshots: + if not snapshots.exists(): logger.error("No completed snapshots found for key: %s", snapshot_key) return
75-76: Add try-finally block to ensure cleanup runs even on errors.Currently,
cleanup()is called after video generation without error handling. Ifgenerate_video()raises an exception, cleanup won't execute, potentially leaving temporary files behind.🔎 Suggested improvement
- video_path = generator.generate_video(video_output_dir, f"{snapshot_key}_snapshot") - generator.cleanup() + try: + video_path = generator.generate_video(video_output_dir, f"{snapshot_key}_snapshot") + finally: + generator.cleanup()This ensures cleanup runs regardless of whether video generation succeeds or fails.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignorebackend/apps/owasp/Makefilebackend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/Makefile
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:296-296
Timestamp: 2025-12-28T15:20:59.650Z
Learning: In `backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py`, the SlideBuilder is intentionally initialized with the module constant OUTPUT_DIR (temp directory) rather than the user-specified output_dir flag. This is by design: intermediate files (images, audio, per-slide videos) are generated in temp, while the final merged video is saved to the user-specified output directory.
📚 Learning: 2025-12-28T15:20:59.650Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:296-296
Timestamp: 2025-12-28T15:20:59.650Z
Learning: In `backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py`, the SlideBuilder is intentionally initialized with the module constant OUTPUT_DIR (temp directory) rather than the user-specified output_dir flag. This is by design: intermediate files (images, audio, per-slide videos) are generated in temp, while the final merged video is saved to the user-specified output directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-08-22T06:36:42.593Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2089
File: backend/apps/nest/models/google_account_authorization.py:61-62
Timestamp: 2025-08-22T06:36:42.593Z
Learning: In the OWASP/Nest project, ruff linting rules discourage using logger.error() immediately before raising exceptions as it creates redundant logging. The preferred approach is to remove the logging call and let the caller handle logging the exception appropriately.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
🔇 Additional comments (1)
.gitignore (1)
30-30: ✅ Appropriate ignore pattern for generated video artifacts.Adding
backend/generated_videos/correctly excludes generated video output from version control. The placement alongside other backend artifacts (backup*,staticfiles) is logical and maintains readability.
backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py
Show resolved
Hide resolved
|
one more change, I'll use |
|
2 participants
Proposed change
Resolves #2924
Checklist
make check-testlocally and all tests passed