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

Expose programmatic equivalent of CLI commands #1854

Merged
merged 48 commits into from
Nov 24, 2023

Conversation

araghukas
Copy link
Contributor

@araghukas araghukas commented Nov 20, 2023

Implements feature requested in #1853

edit: UX updated to

import covalent as ct

# start covalent
ct.covalent_start()

# stop covalent
ct.covalent_stop()
  • I have added the tests to cover my changes.
  • I have updated the documentation and CHANGELOG accordingly.
  • I have read the CONTRIBUTING document.

@araghukas araghukas requested a review from a team as a code owner November 20, 2023 20:18
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #1854 (1daecca) into develop (21f8c0c) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1854      +/-   ##
===========================================
+ Coverage    83.74%   83.80%   +0.06%     
===========================================
  Files          291      293       +2     
  Lines        14316    14363      +47     
  Branches       195      195              
===========================================
+ Hits         11989    12037      +48     
+ Misses        2193     2192       -1     
  Partials       134      134              
Flag Coverage Δ
Dispatcher 90.82% <ø> (ø)
Functional_Tests 51.29% <31.91%> (-0.08%) ⬇️
SDK 79.01% <100.00%> (+0.15%) ⬆️
UI_Backend 85.07% <ø> (ø)
UI_Frontend 73.35% <ø> (ø)

Copy link
Member

@santoshkumarradha santoshkumarradha left a comment

Choose a reason for hiding this comment

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

gtg @araghukas few documentation asks. Can you expose these in the docs API folder please?

and also test 🙏

covalent/_programmatic/commands.py Show resolved Hide resolved
covalent/_programmatic/commands.py Outdated Show resolved Hide resolved
@santoshkumarradha
Copy link
Member

@sriranjanivenkatesan can we pick this up and add it in the docs website please in API section?

@cjao
Copy link
Contributor

cjao commented Nov 21, 2023

Quick question @araghukas -- with this approach, does import covalent pull in covalent_dispatcher?

cjao
cjao previously requested changes Nov 21, 2023
Copy link
Contributor

@cjao cjao left a comment

Choose a reason for hiding this comment

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

Instead of directly importing server-side code, would something like subprocess.run(["covalent", "start"]) work?

covalent/__init__.py Outdated Show resolved Hide resolved
@araghukas
Copy link
Contributor Author

Quick question @araghukas -- with this approach, does import covalent pull in covalent_dispatcher?

@cjao Yes it does introduce a covalent <- covalent_dispatcher dependency at the moment -- but @kessler-frost will fix that soon with in-function (instead of gloal) imports.

@araghukas araghukas marked this pull request as ready for review November 23, 2023 16:13
@araghukas
Copy link
Contributor Author

araghukas commented Nov 23, 2023

Marking ready for review since the added tests are indeed passing now (tests/covalent_tests/programmatic/commands_test.py)

Test failures now are due to UI backend tests...

Is this expected @kessler-frost?

@kessler-frost
Copy link
Member

Yep, I have fixed them in this PR: #1856, @AlejandroEsquivel will merge it soon, then we can merge this as well.

Copy link
Member

@kessler-frost kessler-frost left a comment

Choose a reason for hiding this comment

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

Removing approval for now, needs some discussion.

@santoshkumarradha santoshkumarradha enabled auto-merge (squash) November 24, 2023 20:06
@santoshkumarradha santoshkumarradha dismissed cjao’s stale review November 24, 2023 20:07

Already approved by Casey

@santoshkumarradha santoshkumarradha merged commit 85bd30d into develop Nov 24, 2023
14 checks passed
@santoshkumarradha santoshkumarradha deleted the 1853-programmatic-cli-commands branch November 24, 2023 20:07
@santoshkumarradha
Copy link
Member

@araghukas just checked this in google colab with latest pre-release, does not seem to be working 👀

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.

4 participants