Use FastAPI to provide rest endpoints #135

rosesyrett · 2023-04-21T12:23:18Z

resolves #120

The only thing left to do is change the cli commands.

codecov · 2023-04-21T12:28:28Z

Codecov Report

Merging #135 (a724fab) into main (3cc3d19) will increase coverage by 4.30%.
The diff coverage is 86.61%.

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   83.09%   87.39%   +4.30%     
==========================================
  Files          39       40       +1     
  Lines        1035     1079      +44     
==========================================
+ Hits          860      943      +83     
+ Misses        175      136      -39

Impacted Files	Coverage Δ
src/blueapi/__init__.py	`100.00% <ø> (ø)`
src/blueapi/service/handler.py	`69.76% <69.76%> (ø)`
src/blueapi/service/main.py	`87.87% <87.87%> (ø)`
src/blueapi/cli/cli.py	`97.40% <100.00%> (+33.46%)`	⬆️
src/blueapi/config.py	`95.74% <100.00%> (+0.39%)`	⬆️
src/blueapi/service/__init__.py	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

src/blueapi/cli/cli.py

src/blueapi/config.py

src/blueapi/cli/amq.py

src/blueapi/cli/cli.py

src/blueapi/rest/handler.py

src/blueapi/rest/main.py

src/blueapi/worker/reworker.py

rosesyrett · 2023-04-21T15:59:36Z

Just so you know I don't have write access so can't resolve the merge conflicts. Happy for someone else to do it or give me write access (or both?)

callumforrester

Looking good, needs a bit of back and forth but definitely getting there.

src/blueapi/cli/cli.py

src/blueapi/cli/amq.py

src/blueapi/cli/cli.py

src/blueapi/rest/handler.py

src/blueapi/rest/main.py

src/blueapi/worker/reworker.py

src/blueapi/rest/handler.py

rosesyrett · 2023-04-25T11:15:16Z

I think I've changed stuff enough now, in response to comments. Please have a look.

I'm aware that #145 is related to this PR, i.e. it should probably get merged first and then the lines changed for this PR will be smaller. Happy to work on this with you @callumforrester

src/blueapi/cli/cli.py

callumforrester · 2023-04-25T15:01:52Z

@RAYemelyanova Regarding the merge conflicts, you should have access now, let me know if there are any problems

…ngs. WIP.

callumforrester

Getting close, main thing is we can't have a top-level command called run. I also agree that bluesky worker was the wrong choice which is why I renamed it to blueapi worker, but happy to discuss further if you have objections to that.

.devcontainer/Dockerfile

.vscode/launch.json

src/blueapi/cli/cli.py

src/blueapi/config.py

pyproject.toml

…d deprecation message for worker.

src/blueapi/cli/cli.py

src/blueapi/service/handler.py

src/blueapi/service/main.py

tests/service/test_rest_api.py

tests/test_cli.py

… .json() instead of literal_eval in tests

tpoliaw · 2023-05-09T14:13:02Z

src/blueapi/service/handler.py

+        self.context.with_startup_script(self.config.env.startup_script)
+
+        self.worker = RunEngineWorker(self.context)
+        self.message_bus = StompMessagingTemplate.autoconfigured(self.config.stomp)


I think something needs to handle the stomp connection not being available. At the moment, running without aMQ running prints a stacktrace, then hangs. It'd be good to show a useful error message then exit.

Good idea; will write something in for that.

I agree but think we should sort this out in a separate PR to avoid scope creep.

src/blueapi/cli/cli.py

pyproject.toml

…tures for tests

rosesyrett · 2023-05-09T16:30:06Z

just need to write some tests for the handler to increase code coverage

src/blueapi/cli/cli.py

src/blueapi/cli/fake_cli.py

…d error message for deprecated worker command

src/blueapi/service/main.py

Co-authored-by: Callum Forrester <[email protected]>

rosesyrett · 2023-05-11T09:54:39Z

This is ready to be merged, but here are a few more ideas for future PRs:

the service/handler.py::Handler should gracefully exit if a stomp connection cannot be established,
Need to deal with correlation ID's
Maybe don't use a global HANDLER, but for this some restructuring might be necessary.

rosesyrett mentioned this pull request Apr 21, 2023

add catalog-info for backstage #137

Merged

DiamondJoseph requested changes Apr 21, 2023

View reviewed changes

DiamondJoseph mentioned this pull request Apr 21, 2023

Allow querying for status of in progress and finished tasks #140

Closed

callumforrester requested changes Apr 24, 2023

View reviewed changes

callumforrester reviewed Apr 25, 2023

View reviewed changes

src/blueapi/rest/handler.py Outdated Show resolved Hide resolved

rosesyrett requested review from callumforrester and DiamondJoseph April 25, 2023 11:14

callumforrester reviewed Apr 25, 2023

View reviewed changes

src/blueapi/cli/cli.py Outdated Show resolved Hide resolved

DiamondJoseph mentioned this pull request May 3, 2023

Add support for AMQP over RabbitMQ broker #107

Closed

2 tasks

rosesyrett mentioned this pull request May 3, 2023

Simplify config #169

Merged

rosesyrett force-pushed the restify branch 3 times, most recently from 3bb6d6a to ef7f7db Compare May 5, 2023 12:51

Rose Yemelyanova added 2 commits May 5, 2023 12:52

simple fastapi setup

5e62fb3

squashing commits; rebased onto master and modified some vscode setti…

2c56053

…ngs. WIP.

rosesyrett force-pushed the restify branch from ef7f7db to 2c56053 Compare May 5, 2023 12:57

Rose Yemelyanova added 2 commits May 5, 2023 14:16

made application compatible with config changes

2e539f5

fixed linting error

db7e3cb

callumforrester requested changes May 5, 2023

View reviewed changes

Rose Yemelyanova added 3 commits May 5, 2023 14:32

moved starting the app into blueapi/service/main instead of cli. Adde…

16db22a

…d deprecation message for worker.

moved starting the app into blueapi/service/main instead of cli. Adde…

c2670dd

…d deprecation message for worker.

responded to comments

156d418

rosesyrett requested a review from callumforrester May 5, 2023 14:50

Rose Yemelyanova added 3 commits May 5, 2023 16:03

added some cli tests. Intend to add more...

c407043

added mock as dependency to pass tests

f65425e

added more tests for cli and modified existing rest tests

236a08f

Rose Yemelyanova added 3 commits May 9, 2023 10:51

removed redundant comment

68b7330

removed redundant string concatenation in test

1a26213

moved assert statement

e9cccbe

keithralphs reviewed May 9, 2023

View reviewed changes

made minor changes in response to comments; using functools wraps and…

a702f5f

… .json() instead of literal_eval in tests

tpoliaw reviewed May 9, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Rose Yemelyanova added 5 commits May 9, 2023 15:45

fixed minor issues; param ingestion for cli.py::run_plan and made fix…

5450bd1

…tures for tests

moved common parts of tests to conftest.py, made session scope fixtures

c060e93

fixed linting

6d1119a

made minor changes in response to comments; changed dependencies

a2731f3

modified pyproject toml to include all fastapi dependencies

486cc6c

tpoliaw reviewed May 11, 2023

View reviewed changes

src/blueapi/cli/cli.py Outdated Show resolved Hide resolved

src/blueapi/cli/fake_cli.py Outdated Show resolved Hide resolved

Rose Yemelyanova added 2 commits May 11, 2023 08:52

removed redundant fake_cli file which I just used for testing. Change…

6c0cce4

…d error message for deprecated worker command

fixing tests

5cbba67

callumforrester reviewed May 11, 2023

View reviewed changes

src/blueapi/service/main.py Show resolved Hide resolved

callumforrester reviewed May 11, 2023

View reviewed changes

src/blueapi/service/main.py Outdated Show resolved Hide resolved

callumforrester reviewed May 11, 2023

View reviewed changes

src/blueapi/service/main.py Show resolved Hide resolved

Rose Yemelyanova added 2 commits May 11, 2023 09:34

added extra test for the handler

75c6cdd

made suggested changes

8a633c2

callumforrester reviewed May 11, 2023

View reviewed changes

src/blueapi/service/main.py Outdated Show resolved Hide resolved

Update src/blueapi/service/main.py

a724fab

Co-authored-by: Callum Forrester <[email protected]>

rosesyrett requested a review from callumforrester May 11, 2023 09:56

callumforrester approved these changes May 11, 2023

View reviewed changes

DiamondJoseph approved these changes May 11, 2023

View reviewed changes

modified docs

3863f94

rosesyrett merged commit 76a4e02 into DiamondLightSource:main May 11, 2023

rosesyrett deleted the restify branch May 11, 2023 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use FastAPI to provide rest endpoints #135

Use FastAPI to provide rest endpoints #135

rosesyrett commented Apr 21, 2023

codecov bot commented Apr 21, 2023 •

edited

Loading

rosesyrett commented Apr 21, 2023

callumforrester left a comment

rosesyrett commented Apr 25, 2023

callumforrester commented Apr 25, 2023

callumforrester left a comment

tpoliaw May 9, 2023

rosesyrett May 9, 2023

callumforrester May 11, 2023

rosesyrett commented May 9, 2023

rosesyrett commented May 11, 2023

Use FastAPI to provide rest endpoints #135

Use FastAPI to provide rest endpoints #135

Conversation

rosesyrett commented Apr 21, 2023

codecov bot commented Apr 21, 2023 • edited Loading

Codecov Report

rosesyrett commented Apr 21, 2023

callumforrester left a comment

Choose a reason for hiding this comment

rosesyrett commented Apr 25, 2023

callumforrester commented Apr 25, 2023

callumforrester left a comment

Choose a reason for hiding this comment

tpoliaw May 9, 2023

Choose a reason for hiding this comment

rosesyrett May 9, 2023

Choose a reason for hiding this comment

callumforrester May 11, 2023

Choose a reason for hiding this comment

rosesyrett commented May 9, 2023

rosesyrett commented May 11, 2023

codecov bot commented Apr 21, 2023 •

edited

Loading