fix(gcs): add stored_response_path to sent items #697
Conversation
WalkthroughThe update modifies the logic for uploading files to Google Cloud Storage for items of type 'url'. In addition to uploading 'screenshot_path', the code now also uploads 'stored_response_path' if present. The rest of the upload process, including threading and URI replacement, remains unchanged. Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
secator/hooks/gcs.py (1)
41-46: 🛠️ Refactor suggestionRepeated
storage.Client()creation in every thread is expensive.Each call to
upload_blobinstantiates a newstorage.Client()(line 42).
For large crawls this can spawn hundreds of client objects, increasing memory usage and authentication overhead.
Instantiate a single client once and share it across threads:+# Create a global, thread-safe GCS client once +_GCS_CLIENT = storage.Client() + def upload_blob(bucket_name, source_file_name, destination_blob_name): @@ - storage_client = storage.Client() - bucket = storage_client.bucket(bucket_name) + bucket = _GCS_CLIENT.bucket(bucket_name)
🧹 Nitpick comments (1)
secator/hooks/gcs.py (1)
13-15: Great to seestored_response_pathbeing uploaded as well. Consider making the mapping immutable.The change correctly adds
"stored_response_path"to the list, ensuring responses are shipped together with screenshots.
BecauseITEMS_TO_SENDis meant to be a constant look-up table, replacing the inner lists with tuples avoids accidental mutation at runtime:-ITEMS_TO_SEND = { - 'url': ['screenshot_path', 'stored_response_path'] -} +ITEMS_TO_SEND = { + 'url': ('screenshot_path', 'stored_response_path'), +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing fil 8000 es that changed from the base of the PR and between a785306 and be86cc0.
📒 Files selected for processing (1)
secator/hooks/gcs.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: template (3.11, ubuntu-latest)
- GitHub Check: integration (3.11, ubuntu-latest)
- GitHub Check: unit (3.11)
- GitHub Check: unit (3.8)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (actions)
🔇 Additional comments (1)
secator/hooks/gcs.py (1)
32-35: Potential race:self.threadsmight not be initialised.
process_itemappends toself.threads, but nothing in this file guarantees that the attribute exists.
Ensure the calling class initialisesself.threads = []before any hook runs, or guard against the missing attribute:- t.start() - self.threads.append(t) + t.start() + getattr(self, "threads", []).append(t)
🤖 I have created a release *beep* *boop* --- ## [0.16.4](v0.16.3...v0.16.4) (2025-06-13) ### Bug Fixes * **dalfox:** reduce chunk size for dalfox ([#700](#700)) ([c14be68](c14be68)) * **gcs:** add stored_response_path to sent items ([#697](#697)) ([7c6f992](7c6f992)) * mongodb optimizations ([#699](#699)) ([c0497a6](c0497a6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary by CodeRabbit