Allow configuring IIS log record limit#23
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #23 +/- ##
==========================================
+ Coverage 86.40% 91.54% +5.14%
==========================================
Files 5 5
Lines 206 213 +7
Branches 35 38 +3
==========================================
+ Hits 178 195 +17
+ Misses 20 8 -12
- Partials 8 10 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR removes the hardcoded 1 million record limit from the IIS log parser and adds a configurable -MaxRecords parameter to the PowerShell cmdlet. By default, the parser now reads all records unless explicitly limited.
Key changes:
- Changed default
MaxFileRecord2Readfrom 1,000,000 toint.MaxValue(unlimited) - Added
-MaxRecordsparameter toGet-IISParsedLogcmdlet with validation - Added tests covering both unlimited and limited record parsing scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| IISParser/ParserEngine.cs | Updates default max record limit to int.MaxValue and adds logic to handle zero/negative values as unlimited |
| IISParser.PowerShell/CmdletGetIISParsedLog.cs | Adds new MaxRecords parameter with range validation and documentation example |
| IISParser.Tests/ParserEngineTests.cs | Adds tests for unlimited and capped record parsing with large log files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| MissingRecords = false; | ||
| using var fileStream = File.Open(FilePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); | ||
| using var reader = new StreamReader(fileStream); | ||
| var maxRecords = MaxFileRecord2Read <= 0 ? int.MaxValue : MaxFileRecord2Read; |
There was a problem hiding this comment.
The MaxFileRecord2Read limit is only applied in LongProcess but not in QuickProcess. This means that for log files smaller than 50MB, the MaxRecords parameter will be ignored and all records will be read regardless of the limit set. The QuickProcess method should also respect the MaxFileRecord2Read limit to ensure consistent behavior across all file sizes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| MissingRecords = false; | ||
| using var fileStream = File.Open(FilePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); | ||
| using var reader = new StreamReader(fileStream); | ||
| var maxRecords = MaxFileRecord2Read <= 0 ? int.MaxValue : MaxFileRecord2Read; |
There was a problem hiding this comment.
The new MaxRecords limit is only applied in LongProcess (lines 95‑99), but ParseLog routes files under 50 MB through QuickProcess, which never checks MaxFileRecord2Read. For typical IIS logs (<50 MB), -MaxRecords therefore returns every record instead of stopping after the requested count, so the new example in the cmdlet docs cannot be fulfilled for common log sizes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
-MaxRecordsparameter toGet-IISParsedLogwith documentation for limiting reads when desiredTesting
Codex Task