Skip to content

Fix import_zips#205

Merged
Mab879 merged 2 commits intostigaview:masterfrom
Mab879:cy25q4_update
Jan 26, 2026
Merged

Fix import_zips#205
Mab879 merged 2 commits intostigaview:masterfrom
Mab879:cy25q4_update

Conversation

@Mab879
Copy link
Collaborator

@Mab879 Mab879 commented Dec 3, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved ZIP import parsing to correctly extract product, version, and release; non-matching files are now skipped.
    • Added validation to ignore unknown products and reliably determine short-version labels during import, improving accuracy and robustness.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

📝 Walkthrough

Walkthrough

Replaced an incorrect re.matches use with a proper regex match (product_regex_matches), added a guard to skip non-matching filenames, extracted matches_dict = product_regex_matches.groupdict(), validated product against disa_to_shortname, and updated downstream usage of short_name/short_version in utils/import_zips.py.

Changes

Cohort / File(s) Summary
Import ZIP filename parsing
utils/import_zips.py
Replaced erroneous re.matches assignment with product_regex_matches (regex match object). Added guard to skip non-matching filenames, extracted matches_dict = product_regex_matches.groupdict(), validated product via disa_to_shortname, and updated downstream references to use matches_dict (product, version, release, short_name, short_version). Also replaced sys.exit(main()) with SystemExit(main()) at entry point.

Sequence Diagram(s)

(Skipped — change is confined to a single script file and does not introduce multi-component sequential flows that require visualization.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Aug 2025 update #178 — Modifies utils/import_zips.py around filename regex matching and product-filtering logic; likely overlapping changes.
  • Add CY25Q4 Updates #202 — Updates disa_to_shortname and filename-parsing usage in utils/import_zips.py, directly related to the product mapping and validation added here.

Poem

A rabbit hops through tangled strings, 🐇
Finds the match and trims the things,
Groups aligned in tidy rows,
Unknown products turned to "noes",
Short names hop where parsing goes.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix import_zips' is vague and generic, lacking specificity about what bug or issue is being fixed in the import_zips.py file. Consider a more descriptive title such as 'Fix regex matching in import_zips for product validation' to clarify the specific nature of the fix.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
utils/import_zips.py (3)

61-71: Regex matching & groupdict usage are correct; consider tightening the match

This fix correctly uses a match object with a guard and groupdict(), and the control flow for non-matching filenames looks solid. If you want to avoid accidentally matching filenames that only contain the pattern as a substring, consider using re.match instead of re.search:

-            product_regex_matches = re.search(product_regex, current_zip)
+            product_regex_matches = re.match(product_regex, current_zip)

This keeps the behavior strict (pattern must start at the beginning of the filename) while preserving your new guard logic.


75-82: Minor readability tweak around matches_dict["product"]

The membership check and mapping are correct, but you can make this a bit clearer and avoid repeating the same dictionary lookup:

-                        if matches_dict["product"] not in disa_to_shortname:
+                        product = matches_dict["product"]
+                        if product not in disa_to_shortname:
                             print(
-                                f"skipping {matches_dict['product']} as it is not in known products"
+                                f"skipping {product} as it is not in known products"
                             )
                             continue
-                        short_name = disa_to_shortname[matches_dict["product"]]
+                        short_name = disa_to_shortname[product]

Purely cosmetic, but it tightens the code a bit.


103-104: Simplify program exit by dropping redundant raise

raise sys.exit(main()) works only because sys.exit() itself raises SystemExit, so the raise is effectively dead code. You can simplify this to the usual pattern:

-if __name__ == "__main__":
-    raise sys.exit(main())
+if __name__ == "__main__":
+    sys.exit(main())

Same behavior, clearer intent.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d43cc39 and 593512d.

📒 Files selected for processing (1)
  • utils/import_zips.py (1 hunks)
⏰ 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: deploy

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 593512d and e5ea319.

📒 Files selected for processing (1)
  • utils/import_zips.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
utils/import_zips.py (1)
stigaview_static/models.py (1)
  • short_version (78-79)
⏰ 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: deploy
🔇 Additional comments (3)
utils/import_zips.py (3)

61-67: LGTM! Regex matching fix is correct.

The change properly creates a match object and adds a guard clause to skip non-matching filenames. This correctly fixes the regex matching logic.


103-103: LGTM! Standard exit pattern.

Using raise SystemExit(main()) is a standard and explicit way to exit with the return code from main().


68-72: No issue - version formats are correctly separated by purpose.

The code uses lowercase v{version}r{release} for TOML keys and filenames, which matches the actual product.toml structure ([stigs.v1r1], etc.). The uppercase format in models.py (V{self.version}R{self.release}) is for object representation and is explicitly converted to lowercase when used for URLs (line 83: .short_version.lower()). These formats don't conflict because they serve different layers—the lowercase format is used consistently for all file/TOML operations, while the uppercase format is isolated to the Stig object for display purposes.

Likely an incorrect or invalid review comment.

@Mab879 Mab879 merged commit 8c73fcb into stigaview:master Jan 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant