Skip to content

Extend remoteapprest to add startupdata option#566

Merged
robertopreste merged 3 commits into
masterfrom
565-extend-rest-api
Sep 7, 2021
Merged

Extend remoteapprest to add startupdata option#566
robertopreste merged 3 commits into
masterfrom
565-extend-rest-api

Conversation

@robertopreste

Copy link
Copy Markdown
Contributor

Closes #565

The remoteapprest app start CLI command now accept a --startupdata option to provide a file path that will be loaded at startup.

Note: the URL returned by the remoteapprest app start command cannot be used to open the container yet; there are two issues with it:

  • the port referred in the URL is not correct
  • the URL contains an extra /api/v1/ that needs to be removed

The correct URL is the one printed in the logs; working on a way to return it through remoteapprest.

@codecov-commenter

codecov-commenter commented Sep 3, 2021

Copy link
Copy Markdown

Codecov Report

Merging #566 (d775ddd) into master (9ad2c6f) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head d775ddd differs from pull request most recent head 2f7bc26. Consider uploading reports for the commit 2f7bc26 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #566      +/-   ##
==========================================
+ Coverage   95.16%   95.21%   +0.04%     
==========================================
  Files          92       92              
  Lines        4222     4261      +39     
  Branches      271      273       +2     
==========================================
+ Hits         4018     4057      +39     
  Misses        144      144              
  Partials       60       60              
Impacted Files Coverage Δ
remoteappmanager/cli/remoteapprest/__main__.py 87.60% <100.00%> (+1.23%) ⬆️
remoteappmanager/cli/tests/test_remoteapprest.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ad2c6f...2f7bc26. Read the comment docs.

@flongford flongford left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall looks good, thanks @robertopreste - just a couple of comments

Comment thread remoteappmanager/cli/remoteapprest/__main__.py
Comment thread remoteappmanager/cli/tests/test_remoteapprest.py
@robertopreste

Copy link
Copy Markdown
Contributor Author

@flongford I fixed this, so that now the CLI would first check the allow_startup_data policy, and start the container with the given data only if that policy is True.

@flongford flongford left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@robertopreste robertopreste merged commit c5748e6 into master Sep 7, 2021
@robertopreste robertopreste deleted the 565-extend-rest-api branch September 7, 2021 09:16
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.

Expand Simphony-Remote start command in rest API to include startupdata

3 participants