Restore provenance action ver field format
#112
Conversation
This restores the original Java server `ver` field format of `<module version>-<relase version>`. It provides more information for the user and allows the kb_sdk_plus tests to pass.
There was a problem hiding this comment.
Pull Request Overview
This PR restores the original Java server format for the provenance action ver field, changing it from just the service version to a combined format of <module version>-<service version>. This change provides more detailed version information and ensures compatibility with kb_sdk_plus tests.
- Updates the
verfield format in MethodRunner.py to include both module and service versions - Adds comprehensive test coverage for provenance functionality in the callback server integration tests
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| JobRunner/MethodRunner.py | Updates the provenance action ver field to combine module version and service version |
| test/test_callback_server_integration.py | Expands existing test to include provenance setting and validation |
| "version": "1.1" | ||
| } | ||
|
|
||
| resp = _post(port, {"method":"CallbackServer.get_provenance"}) |
There was a problem hiding this comment.
Missing space after colon in dictionary literal. Should be {"method": "CallbackServer.get_provenance"}
| resp = _post(port, {"method":"CallbackServer.get_provenance"}) | |
| resp = _post(port, {"method": "CallbackServer.get_provenance"}) |
| 'code_url': 'https://github.com/kbasetest/njs_sdk_test_1', | ||
| 'commit': '366eb8cead445aa3e842cbc619082a075b0da322', | ||
| 'name': 'njs_sdk_test_1', | ||
| 'ver': '0.0.3-dev' |
There was a problem hiding this comment.
Wait, it can't have a version and a dev at the same time, can it? I thought in order to have a version it must be beta or release?
There was a problem hiding this comment.
No, it always has a version and something to identify the release info - either release/dev/beta or a commit
There was a problem hiding this comment.
So what would ver look like if it wasn't released?
There was a problem hiding this comment.
Whatever the version is in the kbase.yml file
There was a problem hiding this comment.
So 0.0.3-dev and 0.0.3-beta and 0.0.3-prod could all have the same commit or a different commit depending on the registration status in the catalog.. So what is the point of this field? Shouldn't we just be returning the git commit hash?
There was a problem hiding this comment.
So 0.0.3-dev and 0.0.3-beta and 0.0.3-prod could all have the same commit or a different commit depending on the registration status in the catalog.
For dev that's true, beta and release would have to have the same git commit I think. IIRC the catalog forces a version bump on any change
So what is the point of this field?
It helps the user understand what code ran and is a quick check along those lines. It's a lot easier to figure out the version of a module from the catalog UI given the semantic version than trying to figure it out from the git commit. It also is monotonic which again helps figuring out how old the module is relative to the most recent release
Shouldn't we just be returning the git commit hash?
The info above is redundant to the git commit, yes, but it's helpful for humans as outlined above
There was a problem hiding this comment.
For dev that's true, beta and release would have to have the same git commit I think. IIRC the catalog forces a version bump on any change
Nope, I'm wrong, you could move a new version to beta but not release
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
=======================================
Coverage 81.03% 81.03%
=======================================
Files 13 13
Lines 1155 1155
=======================================
Hits 936 936
Misses 219 219 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Codacy is complaining about asserts in tests |
|
@bio-boris confirmed out of band that this is ok to merge |
This restores the original Java server
verfield format of<module version>-<relase version>. It provides more information for the user and allows the kb_sdk_plus tests to pass.