WIP: Implement basic add command #177

0101coding · 2022-09-21T09:54:22Z

This is draft PR for Issue #9

cnpryer · 2022-09-21T10:56:29Z

Thanks! I should have more time later today to check this out.

cnpryer

A few comments related to how were implementing this and potentially some final steps. Besides what would be the remaining cleanup, this looks like a good first pass!

Thanks for helping out!

cnpryer · 2022-09-21T14:24:20Z

Cargo.toml

-
+serde_json = "1.0"
+reqwest = { version = "0.11", features = ["blocking", "json"] } 
+typer = "0.1.1"


Are we using this?

We are not using typer. but we are using serde_json and reqwests

Okay cool. Then we can remove this.

cnpryer · 2022-09-21T14:30:37Z

src/bin/huak/commands/add.rs

+    let path = format!("https://pypi.org/pypi/{}/json", dependency.unwrap());
+
+    //info!("Requesting data from {}", path);
+    let resp: reqwest::blocking::Response = reqwest::blocking::get(path)?;
+
+    let json: PyPi = resp.json()?;
+    println!("{:#?}", json);
+
+    // Get the version
+
+    let version = json.info.version;
+    let name = json.info.name;
+    let dep = PythonPackage{
+        name,
+        version,
+    }; 
+
+    //let cwd = cwd_buff.as_path();
+    let venv = Venv::default();
+    venv.install_package(&dep)?;


A few comments on this part.

Can we move this to huak::ops::add? In bin/huak/commands/ we're only adding a thin CLI layer and implementing all the operation logic as part of the lib. So you'd want to

Suggested change

let path = format!("https://pypi.org/pypi/{}/json", dependency.unwrap());

//info!("Requesting data from {}", path);

let resp: reqwest::blocking::Response = reqwest::blocking::get(path)?;

let json: PyPi = resp.json()?;

println!("{:#?}", json);

// Get the version

let version = json.info.version;

let name = json.info.name;

let dep = PythonPackage{

name,

version,

};

//let cwd = cwd_buff.as_path();

let venv = Venv::default();

venv.install_package(&dep)?;

let project = Project::from(&cwd);

ops::add::add_dependency_to_project(&project)?;

If we don't need certain annotations because the compiler will imply them, then maybe we remove those too. I'm not sure if PyPI can be implied but I wonder if the Response can.

From the ops space, we want to standardize operations to execute on Projects vs Venvs. So you should be able to do the same thing here.

Last thing would be adding to the Toml.

Also let's make sure to remove and prints/debug code before a merge.

src/huak/errors.rs

cnpryer · 2022-09-21T14:41:02Z

Looks like we also need to make sure this passes the ci-rust action.

0101coding · 2022-09-22T07:35:05Z

I will update my branch from remote so that the repo is in sync with mainstream. Will look at the ci-rust actions too

0101coding · 2022-09-23T19:55:33Z

Hi @cnpryer after making the changes, i ran cargo run add typer, and it shows what it is in the screenshot, however I cannot see any file tracking installed dependency

Any hint will be appreciated

cnpryer · 2022-09-23T20:28:40Z

Hmm I'm not sure what you mean. Can you give me something to reproduce this with?

Cargo.toml

cnpryer · 2022-09-24T14:50:00Z

Hi! I'm hoping to get to PyPI this week or next weekend the latest. How are you doing here? I see that you're on your master, so unfortunately I don't think I can help out unless I PR your master. Correct me if I'm wrong.

It looks like you need a rebase. I would rebase when you can. I want to update PythonPackage to include something like

pub struct PythonPackage {
    string: String, // TODO: More like a view maybe.
    pub name: String,
    pub op: Option<String>,
    pub version: Option<String>,
}

impl PythonPackage {
    pub fn new(name: String, op: Option<String>, version: Option<String>) -> PythonPackage {
        let string = py_package_string_from_parts(&name, &op, &version);

        PythonPackage { string, name, op, version }
    }

    pub fn from(string: String) -> PythonPackage {
       let (name, op, version) = py_package_from_string(string);

       PythonPackage { string, name, op, version }
    }

    pub fn string(&self) -> &String {
        &self.string
    }

    pub fn set_string(&mut self, name: &str, op: &str, version: &str) {
        self.string = format!("{name}{op}{version}");
    }
}

fn py_package_from_string(string: String) -> (String, Option<String>, Option<String>) {
    ("".to_string(), None, None)
}

fn py_package_string_from_parts(name: &str, op: &Option<String>, version: &Option<String>) -> String {
    "".to_string()
}

If we move to that I think it'd be straightfoward to wrap up your PR, unless I'm missing something. The PythonPackage change would take a little bit of rework. So I can try to get that in today. From there you'd just need to do the following:

Move to huak::ops::add::add_project_dependency, only leave something like this in the bin space

let dependency = match args.get_one::<String>("dependency") {
        Some(d) => d,
        None => {
            return Err(CliError::new(
                anyhow::format_err!("no dependency was provided"),
                2,
            ))
        }
    };

Add a test
Update for anything added from my PythonPackage change

I'll update this comment throughout the day if something changes.

0101coding · 2022-09-24T20:36:01Z

I have made an initial push of the changes made, though test implementation is still outstanding

cnpryer · 2022-09-25T00:26:25Z

src/bin/huak/commands/add.rs

@@ -1,6 +1,7 @@
 use super::utils::subcommand;
 use clap::{value_parser, Arg, ArgMatches, Command};
 use huak::errors::CliResult;
+use huak::ops::add::add_project_dependency;


Suggested change

use huak::ops::add::add_project_dependency;

use huak::ops;

cnpryer · 2022-09-25T00:32:39Z

src/huak/ops/add.rs

+
+pub fn add_project_dependency(
+    package: String,
+    _dependency_is_dev: bool,


Suggested change

_dependency_is_dev: bool,

_is_dev: bool,

cnpryer

Cool. I added the new error pattern here. It's probably not correct yet, but at least gives you an idea how we can handle errors in the bin space as well (post rebase).

Let me know if you need any help. Maybe I'll have the PythonPackage changes done for your test.

cnpryer · 2022-09-25T00:34:34Z

src/huak/ops/add.rs

+    // Get the version
+    let version = json.info.version;
+    let name = json.info.name;
+    let mut dep = PythonPackage::new(name.clone());


we should be able to do

let dep = PythonPackage::new(name, None, version);

after the PythonPackage rewrite.

Should be good to go. It'll default the string to "name==version".

cnpryer · 2022-09-25T00:35:07Z

src/bin/huak/commands/add.rs

+    let dependency = args.get_one::<String>("dependency");
+    add_project_dependency(dependency.unwrap().to_string(), false)?;


Suggested change

let dependency = args.get_one::<String>("dependency");

add_project_dependency(dependency.unwrap().to_string(), false)?;

let dependency = match args.get_one::<String>("dependency") {

Some(d) => d,

None => {

return Err(CliError::new(HuakError::MissingArguments))

}

};

ops::add::add_project_dependency(dependency, false)?;

I'm like 60% sure i got this right. I'll come back.

Only change I think is adding , 1) to just return a basic exit code. A rebase will add status codes to CliError.

cnpryer · 2022-09-25T00:38:03Z

Btw if you need more interactive communication I set up a discord https://discord.gg/St3menxFZT

cnpryer requested changes Sep 21, 2022

View reviewed changes

cnpryer changed the title ~~Draft PR for Add Implementation~~ WIP: Implement basic add command Sep 21, 2022

cnpryer force-pushed the master branch from 3b682c6 to 3de62ff Compare September 23, 2022 16:19

cnpryer reviewed Sep 24, 2022

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

cnpryer reviewed Sep 25, 2022

View reviewed changes

cnpryer mentioned this pull request Sep 25, 2022

Updated PythonPackage initializing functions #212

Merged

0101coding closed this Sep 25, 2022

0101coding force-pushed the master branch from 6845cbc to 0d804e6 Compare September 25, 2022 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Implement basic add command #177

WIP: Implement basic add command #177

0101coding commented Sep 21, 2022 •

edited by cnpryer

Loading

cnpryer commented Sep 21, 2022

cnpryer left a comment

cnpryer Sep 21, 2022

0101coding Sep 22, 2022

cnpryer Sep 23, 2022

cnpryer Sep 21, 2022 •

edited

Loading

cnpryer commented Sep 21, 2022

0101coding commented Sep 22, 2022

0101coding commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer commented Sep 24, 2022 •

edited

Loading

0101coding commented Sep 24, 2022

cnpryer Sep 25, 2022

cnpryer Sep 25, 2022

cnpryer left a comment

cnpryer Sep 25, 2022

cnpryer Sep 25, 2022

cnpryer Sep 25, 2022 •

edited

Loading

cnpryer Sep 25, 2022

cnpryer commented Sep 25, 2022

		let dependency = args.get_one::<String>("dependency");
		add_project_dependency(dependency.unwrap().to_string(), false)?;

WIP: Implement basic add command #177

WIP: Implement basic add command #177

Conversation

0101coding commented Sep 21, 2022 • edited by cnpryer Loading

cnpryer commented Sep 21, 2022

cnpryer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnpryer Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

cnpryer commented Sep 21, 2022

0101coding commented Sep 22, 2022

0101coding commented Sep 23, 2022

cnpryer commented Sep 23, 2022

cnpryer commented Sep 24, 2022 • edited Loading

0101coding commented Sep 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnpryer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnpryer Sep 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnpryer commented Sep 25, 2022

0101coding commented Sep 21, 2022 •

edited by cnpryer

Loading

cnpryer Sep 21, 2022 •

edited

Loading

cnpryer commented Sep 24, 2022 •

edited

Loading

cnpryer Sep 25, 2022 •

edited

Loading