Skip to content

Restore provenance action ver field format #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MrCreosote merged 2 commits into from
Aug 11, 2025
Merged

Restore provenance action ver field format #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7311f9
Restore provenance action `ver` field format
MrCreosote Aug 10, 2025
80ffb3d
Add space
MrCreosote Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion JobRunner/MethodRunner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def run(
# If there is a fin_q then run this async
action = {
"name": module,
"ver": service_ver,
"ver": f"{module_info['version']}-{service_ver}",
"code_url": module_info["git_url"],
"commit": module_info["git_commit_hash"],
}
Expand Down
30 changes: 29 additions & 1 deletion test/test_callback_server_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,19 @@ def test_callback_method_fail_no_method(callback_ports):
}


def test_submit_job_sync(callback_ports):
def test_submit_job_sync_and_provenance(callback_ports):
port = callback_ports[0]

resp = _post(
port,
{
"method": "CallbackServer.set_provenance",
"params": [{"method": "foo.bar", "service_ver": "beta"}]
}
)
j = resp.json()
assert j.keys() == {"result"}

resp = _post(port, {
"method": "njs_sdk_test_1.run",
"params": [{"id": "whee"}],
Expand All @@ -265,6 +275,24 @@ def test_submit_job_sync(callback_ports):
"version": "1.1"
}

resp = _post(port, {"method": "CallbackServer.get_provenance"})
j = resp.json()
del j["result"][0][0]["time"] # changes from run to run
assert j == {'result': [[{
'description': 'KBase SDK method run via the KBase Execution Engine',
'input_ws_objects': [],
'method': 'bar',
'method_params': [],
'service': 'foo',
'service_ver': 'beta',
'subactions': [{
'code_url': 'https://github.com/kbasetest/njs_sdk_test_1',
'commit': '366eb8cead445aa3e842cbc619082a075b0da322',
'name': 'njs_sdk_test_1',
'ver': '0.0.3-dev'
Copy link
Collaborator

@bio-boris bio-boris Aug 11, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

@MrCreosote MrCreosote Aug 11, 2025

Choose a reason for hiding this comment

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

No, it always has a version and something to identify the release info - either release/dev/beta or a commit

Copy link
Collaborator

@bio-boris bio-boris Aug 11, 2025

Choose a reason for hiding this comment

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

So what would ver look like if it wasn't released?

Copy link
Member Author

@MrCreosote MrCreosote Aug 11, 2025

Choose a reason for hiding this comment

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

Whatever the version is in the kbase.yml file

Copy link
Collaborator

@bio-boris bio-boris Aug 11, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

@MrCreosote MrCreosote Aug 11, 2025

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@MrCreosote MrCreosote Aug 11, 2025

Choose a reason for hiding this comment

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

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

}],
}]]}


def test_submit_job_async(callback_ports):
port = callback_ports[0]
Expand Down
Loading