Create build release workflow for MI - #193
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a tag-driven GitHub Release workflow for the MongoSync Insights subproject and adjusts log timestamp bound normalization so SQLite string comparisons behave correctly with stored ...Z timestamps.
Changes:
- Introduces
.github/workflows/mongosync-insights-release.ymlto run tests, build multi-platform artifacts, package MI-only source archives, and publish a GitHub Release onMIv*tags. - Updates timestamp bound normalization (
normalize_search_start/normalize_search_end) and strengthens coverage for edge cases (bareZ, high-millisecond timestamps). - Expands packaging/testing documentation and bumps
APP_VERSIONto0.9.1.16.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| migration/mongosync_insights/tests/TESTING.md | Adds a detailed MI tests inventory and execution guide. |
| migration/mongosync_insights/tests/test_logs_routes.py | Adds a route-level regression test for end-bound timestamp filtering edge cases. |
| migration/mongosync_insights/tests/test_log_time.py | Updates expectations for normalized search bounds (Z-suffixed). |
| migration/mongosync_insights/tests/test_log_store.py | Updates timestamp LTE/range tests and adds coverage for high-millisecond inclusion. |
| migration/mongosync_insights/PACKAGING.md | Documents CI-driven GitHub Releases and published assets. |
| migration/mongosync_insights/lib/log_time.py | Fixes/clarifies timestamp normalization to align with stored ...Z values. |
| migration/mongosync_insights/lib/app_config.py | Bumps MI application version. |
| migration/mongosync_insights/ci/RELEASE_DOWNLOADS.md | Adds release-body template describing correct assets to download. |
| migration/mongosync_insights/ci/package_source_archives.sh | Adds script to generate MI-only source tar/zip assets for releases. |
| migration/mongosync_insights/build_rhel.sh | Adds --el-major option to distinguish el8 vs el9 RPM iterations. |
| migration/mongosync_insights/_build_linux_common.sh | Improves prereq checks and supports RHEL el-major iteration suffix. |
| .github/workflows/mongosync-insights-tests.yml | Updates pinned action revisions for checkout/setup-python. |
| .github/workflows/mongosync-insights-release.yml | New release workflow: tests, builds, artifact aggregation, and GitHub Release publishing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
migration/mongosync_insights/build_rhel.sh:31
--el-majoris documented as accepting only 8 or 9, but the script currently accepts any value and will happily generate iteration strings like1.el10. Validating the argument early prevents accidental mislabeling of RPMs.
--el-major)
RHEL_EL_MAJOR="${2:?--el-major requires 8 or 9}"
shift 2
;;
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
migration/mongosync_insights/build_rhel.sh:31
- The script help text says
--el-majorrequires8or9, but the current parsing accepts any value. Validating the input early avoids producing misleading1.el<value>RPM iterations.
--el-major)
RHEL_EL_MAJOR="${2:?--el-major requires 8 or 9}"
shift 2
;;
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (5)
migration/mongosync_insights/tests/TESTING.md:63
- The per-file test counts in this table are out of sync with the current tests in this PR (e.g.,
test_log_store.pynow has 14 test functions,test_log_time.py7, andtest_logs_routes.py17). Please update these counts (or regenerate the report) so the table matches the actual suite.
| `test_log_store.py` | 11 | Insert, FTS search, pagination, timestamp range queries, delete |
| `test_log_store_registry.py` | 4 | Store open/cache hit-miss, expiry, maintenance cleanup |
| `test_log_time.py` | 5 | Log search datetime parsing, start/end bound normalization |
| `test_file_decompressor.py` | 20 | gzip/bzip2/tar/zip decompression, macOS metadata skipping, MIME routing |
| `test_logs_routes.py` | 15 | `/logs` home, upload, search (text + timestamp), snapshot list/load/delete |
migration/mongosync_insights/tests/TESTING.md:194
- These section headers still show the old test counts for files updated in this PR (log store/time/routes). Keeping these in sync avoids the report contradicting the actual test suite.
### `test_log_store.py` (11)
migration/mongosync_insights/PACKAGING.md:211
- The build scripts invoked by these instructions use
python3and require it to be 3.11+. Installingpython3.11does not guarantee that/usr/bin/python3points to 3.11 on Ubuntu, so following these steps can still fail the prereq check. Consider adding an explicitpython3 --versioncheck (and guidance on switchingpython3to 3.11 if needed).
cd migration/mongosync_insights
sudo apt-get update
sudo apt-get install -y python3.11 python3.11-venv python3-pip ruby-rubygems build-essential
sudo gem install fpm
./build_ubuntu.sh
migration/mongosync_insights/build_ubuntu.sh:10
- These prerequisites mention installing Python 3.11, but the build scripts require the
python3command itself to be 3.11+. On Ubuntu,python3can remain at 3.10 even after installingpython3.11, so it’s helpful to call out verifyingpython3 --version(and adjusting alternatives if needed).
# Prerequisites:
# sudo apt-get update
# sudo apt-get install -y python3.11 python3.11-venv python3-pip ruby-rubygems build-essential
# sudo gem install fpm
migration/mongosync_insights/_build_linux_common.sh:85
- This prereq help suggests installing
python3.11packages, but the build scripts requirepython3itself to be 3.11+. On some systems, installingpython3.11does not automatically makepython3point to it, so it helps to explicitly call out checkingpython3 --versionhere (and adjusting if needed).
sudo apt-get update
sudo apt-get install -y python3.11 python3.11-venv python3-pip ruby-rubygems build-essential
sudo gem install fpm
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
migration/mongosync_insights/tests/TESTING.md:7
- This section hard-codes test/file counts (e.g., 516/30). Those numbers will drift as tests change, which can make the report misleading; consider adding an explicit note that counts are point-in-time and pointing readers at
--collect-onlyfor the current number.
**Total:** **516 tests** across **30 test files** (+ `conftest.py` shared fixtures)
migration/mongosync_insights/lib/log_time.py:5
- Minor wording issue: "millisecond-prefixed strings" reads like the milliseconds are a prefix rather than the precision/format. Tweaking this phrasing makes the contract clearer.
"""ISO-8601 helpers for log timestamp search bounds (literal, no timezone conversion).
Start bounds use millisecond-prefixed strings without a ``Z`` suffix so SQLite
string comparison matches stored mongosync log ``time`` values with or without
``Z``. End bounds use a ``Z`` suffix on the selected second (no fraction).
Fix stored log time Fix documentation Fix automated tests
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/mongosync-insights-release.yml:45
- The embedded
python3 -csnippet is indented inside the quoted string, which will be passed to Python with leading spaces on each line and can raiseIndentationError: unexpected indent. Use a heredoc (or a single-line-c) so the script is indentation-safe in YAML.
app_version=$(python3 -c "
import re, pathlib
text = pathlib.Path('lib/app_config.py').read_text()
m = re.search(r'APP_VERSION\s*=\s*\"([^\"]+)\"', text)
print(m.group(1) if m else '')
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Create build release workflow
Fix stored log time
Fix documentation
Fix automated tests