Skip to content
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.

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

ketchup: json string switch and unready function #80

Merged
merged 16 commits into from
Jun 6, 2024

Conversation

g-kimbell
Copy link

Adds a [-J/--json] switch to ketchup to send/receive json strings

ketchup -J submit [payload]
Expects the payload to be a base64 encoded (to avoid issues with spaces and special characters) json string

ketchup -J status
Returns a json string instead of a formatted table

@g-kimbell
Copy link
Author

ketchup unready [pipeline] is also added and working now

@g-kimbell g-kimbell changed the title ketchup: json string switch ketchup: json string switch and unready function May 23, 2024
Copy link
Contributor

@PeterKraus PeterKraus left a comment

Choose a reason for hiding this comment

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

In general, it's not a bad idea to do a JSON interface instead of a YAML one for tomato-1.0, too. I'm not super happy with the YAML output as it's very bulky.

src/tomato/ketchup/functions.py Outdated Show resolved Hide resolved
if args.payload.endswith("json"):
pldict = json.load(infile)
elif args.payload.endswith("yml") or args.payload.endswith("yaml"):
pldict = yaml.full_load(infile)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why I'm using yaml.full_load() here. Could you check with a safe_load and see if the tests pass?

Copy link
Author

Choose a reason for hiding this comment

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

I can't get the pytest to work properly so can't check this

9/10 tests fail with assert 'cd' == 'c'

Not sure if it is the cause but I see:

INFO:dgbowl_schemas.tomato:Could not parse 'kwargs' using Payload v0.2.
INFO:dgbowl_schemas.tomato:1 validation error for Payload
version
  unexpected value; permitted: '0.2' (type=value_error.const; given=0.1; permitted=('0.2',))
DEBUG:tomato.ketchup.functions:Payload=Payload(version='0.1' tomato=Tomato(unlock_when_done=False, verbosity='WARNING', output=Output(path=None, prefix=None)) sample=Sample(name='dummy_random_5_2') method=[Method(device='worker', technique='random', delay=2, time=5)])

I can send you the full log if you want

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, the test fails with safe_load or fails always with this PR?

Copy link
Author

Choose a reason for hiding this comment

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

Always even without this PR, on one PC I get 1 test passing, on another I get all fails

src/tomato/main.py Outdated Show resolved Hide resolved
src/tomato/ketchup/functions.py Outdated Show resolved Hide resolved
g-kimbell added a commit to g-kimbell/tomato that referenced this pull request Jun 3, 2024
Refactor ketchup status
-J argument only on status and submit functions
@PeterKraus
Copy link
Contributor

PeterKraus commented Jun 4, 2024

@g-kimbell I've pushed a modified workflow file into this branch, so the tests are being run here on github. You'll have to pull the branch first. Let's get the tests fixed before merging. In GH's logs I see:

2024-06-04T08:29:19.1862297Z 2024-06-04 08:28:18,006:INFO    :MainProcess:------------------------------------------
2024-06-04T08:29:19.1863340Z 2024-06-04 08:28:18,006:DEBUG   :data_poller_1_worker:in 'data_poller', pollrate=1
2024-06-04T08:29:19.1864237Z 2024-06-04 08:28:18,007:DEBUG   :data_poller_1_worker:in 'dummy.get_status'
2024-06-04T08:29:19.1865024Z 2024-06-04 08:28:18,006:DEBUG   :data_poller_1_worker:in 'data_poller', pollrate=1
2024-06-04T08:29:19.1865720Z Process data_poller_1_worker:
2024-06-04T08:29:19.1866294Z 2024-06-04 08:28:18,007:DEBUG   :data_poller_1_worker:in 'dummy.get_status'
2024-06-04T08:29:19.1866877Z Traceback (most recent call last):
2024-06-04T08:29:19.1876312Z   File "/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/multiprocessing/process.py", line 315, in _bootstrap
2024-06-04T08:29:19.1877154Z     self.run()
2024-06-04T08:29:19.1877855Z   File "/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/multiprocessing/process.py", line 108, in run
2024-06-04T08:29:19.1878753Z     self._target(*self._args, **self._kwargs)
2024-06-04T08:29:19.1880117Z   File "/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/tomato/drivers/driver_funcs.py", line 149, in data_poller
2024-06-04T08:29:19.1881160Z     mem_size = metadata["mem_size"]
2024-06-04T08:29:19.1881595Z KeyError: 'mem_size'
2024-06-04T08:29:19.1882403Z 2024-06-04 08:28:18,009:DEBUG   :MainProcess:p=<Process name='data_poller_1_worker' pid=1951 parent=1946 stopped exitcode=1>
2024-06-04T08:29:19.1883550Z 2024-06-04 08:28:18,009:CRITICAL:MainProcess:'data_poller' with pid 1951 was terminated with exit code 1
2024-06-04T08:29:19.1884364Z 2024-06-04 08:28:18,009:INFO    :MainProcess:-----------------------

So I'm guessing we broke it in the previous PR - the dummy device doesn't have mem_size.

@g-kimbell
Copy link
Author

Thanks, I will move the memory stuff into biologic main

@g-kimbell
Copy link
Author

Now it breaks because I in data_poller I started using get_data to also return the status, which works for biologic and cuts the API calls by half, but it is not general. Maybe it is best to duct tape fix this with a try except and have a better think about how to implement this in 1.0

@g-kimbell
Copy link
Author

Run sphinx-build -W -b html docs/source public/master
Running Sphinx v4.5.0

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.
Error: Process completed with exit code 2

This one I'm not sure about. We have "sphinx==4.5.0" in setup.py, nothing about applehelp in the repo

@PeterKraus
Copy link
Contributor

Run sphinx-build -W -b html docs/source public/master
Running Sphinx v4.5.0

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.
Error: Process completed with exit code 2

This one I'm not sure about. We have "sphinx==4.5.0" in setup.py, nothing about applehelp in the repo

This looks like an issue with the environment, so not caused by this PR. I'll fix the pages for that branch before the new release.

@PeterKraus
Copy link
Contributor

@g-kimbell The docs should be fixed in #87 on the 0.2.x branch, please rebase (git pull --rebase upstream 0.2.x) and then force-push back to your own branch.

Change get_status and get_data order to avoid race conditions when polling rate is the same as test duration
Copy link
Contributor

@PeterKraus PeterKraus left a comment

Choose a reason for hiding this comment

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

🔋 ♻️ 🪫

@PeterKraus PeterKraus merged commit fb93703 into dgbowl:0.2.x Jun 6, 2024
9 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.

2 participants