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.

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

Jump to bottom

Add proxy apps command line interface #1003

Merged
merged 18 commits into from
Jan 6, 2024
Merged

Add proxy apps command line interface #1003

merged 18 commits into from
Jan 6, 2024

Conversation

romer8
Copy link
Contributor

@romer8 romer8 commented Dec 27, 2023 •
edited
Loading

  • Add function proxyapp with the following flags: list, update, add
  • list method in order to list the current proxy apps
  • add method in order to add a new proxy app
  • update method with flag -s in order to update a current proxy app settings , multiple settings can be changed
  • Test cases with 100%

swainn
swainn requested changes Dec 27, 2023
View reviewed changes
Copy link
Member

@swainn swainn left a comment

Choose a reason for hiding this comment

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

See comments.

tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Dec 27, 2023 •
edited
Loading

Coverage Status

coverage: 100.0%. remained the same
when pulling a6d18a8 on romer8:add_proxy_apps_cli
into b818328 on tethysplatform:main.

swainn
swainn reviewed Dec 28, 2023
View reviewed changes
Copy link
Member

@swainn swainn left a comment

Choose a reason for hiding this comment

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

See additional requests for changes.

tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
swainn
swainn requested changes Jan 2, 2024
View reviewed changes
Copy link
Member

@swainn swainn left a comment

Choose a reason for hiding this comment

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

I reviewed the tests more closely now and added a few comments.

tests/unit_tests/test_tethys_cli/test_proxyapps_commands.py Outdated Show resolved Hide resolved
tests/unit_tests/test_tethys_cli/test_proxyapps_commands.py Outdated Show resolved Hide resolved
tests/unit_tests/test_tethys_cli/test_proxyapps_commands.py Show resolved Hide resolved
tests/unit_tests/test_tethys_cli/test_proxyapps_commands.py Show resolved Hide resolved
tests/unit_tests/test_tethys_cli/test_proxyapps_commands.py Show resolved Hide resolved
tests/unit_tests/test_tethys_cli/test_proxyapps_commands.py Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
@swainn swainn requested review from sdc50 and shawncrawley January 2, 2024 15:16
@swainn
Copy link
Member

swainn commented Jan 2, 2024

@shawncrawley @sdc50 I'd like to have one or both of you review this PR I've been working with @romer8 on.

shawncrawley
shawncrawley reviewed Jan 4, 2024
View reviewed changes
Copy link
Contributor

@shawncrawley shawncrawley left a comment

Choose a reason for hiding this comment

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

I only found one minor cli documentation issue, otherwise this looks great!

tethys_cli/proxyapps_commands.py Outdated Show resolved Hide resolved
shawncrawley
shawncrawley approved these changes Jan 5, 2024
View reviewed changes
Copy link
Contributor

@shawncrawley shawncrawley left a comment

Choose a reason for hiding this comment

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

I am pleased.

@swainn swainn merged commit f5cfc0b into tethysplatform:main Jan 6, 2024
9 of 10 checks passed
@sdc50 sdc50 added the cli label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shawncrawley shawncrawley shawncrawley approved these changes

@swainn swainn swainn approved these changes

@sdc50 sdc50 Awaiting requested review from sdc50

Assignees
No one assigned
Labels
cli minor feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@romer8 @coveralls @swainn @shawncrawley @sdc50