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

[FeatureRequest] List host command #125

Closed
vezdeneszter opened this issue Oct 4, 2023 · 10 comments
Closed

[FeatureRequest] List host command #125

vezdeneszter opened this issue Oct 4, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest Issue to help with during Hacktoberfest2023

Comments

@vezdeneszter
Copy link
Contributor

dem list-host
List the available hosts from the config file.

If there is no host in the config file, inform the user that no remote host availalbe.

@vezdeneszter vezdeneszter added enhancement New feature or request hacktoberfest Issue to help with during Hacktoberfest2023 labels Oct 4, 2023
@janosmurai janosmurai added the good first issue Good for newcomers label Oct 9, 2023
@Kabiirk
Copy link
Contributor

Kabiirk commented Oct 17, 2023

Hi @vezdeneszter ,

Can I work in this ? I have experience with python, but this would be the first time I would be contributing to an actual python package hosted on PIP.

Best Regards.
Kabiir Krishna

@vezdeneszter
Copy link
Contributor Author

Hi, thank you for your offer to contribute! Sure, I'll assign it to you. Please don't forget to join our Discord server: https://discord.com/invite/Nv6hSzXruK where we have a dedicated channel for Hacktoberfest. There you can discuss this issue in detail 😊

@Kabiirk
Copy link
Contributor

Kabiirk commented Oct 17, 2023

Thanks !
appreciate it 👍

@Kabiirk
Copy link
Contributor

Kabiirk commented Oct 23, 2023

Hi Guys,

I was able to get the project up and running. But I am unable to re-build the project from source (is it required ?) to see whatever changes I have made to my local branch. I was also facing some issues in creating a test environment to check my outputs.
image

Changes made till now:
image
I simply added a new cmd_script and some basic code in main.py to run it like other CLI commands. However, when I input dem list-host I get the following error:
image

Let me know if there are any other steps to be performed apart from the ones mentioned in the DEM Contribution Guidelines

Best Regards.
Kabiir Krishna

P.S.
Here is my System info (if it help)
image

@janosmurai
Copy link
Contributor

Hi @Kabiirk
Your approach to the implementation looks good to me.
You installed DEM as a pip package, meaning you don't run it from source. This is the reason why you can't use your new command. First, you need to set up a virtual environment with poetry and you will be able to test you modifications. If you need any help to do that, text me on Discord!
The first error you see means that something is not quite alright in your installation. (The error message you see is not very helpful, so that must be improved in the future.) Could you tell me which Docker Engine version do you have?

@Kabiirk
Copy link
Contributor

Kabiirk commented Oct 23, 2023

Hi @janosmurai ,

Appreciate the prompt revert.

  1. I don't have axem-dem installed via pip:
    image

  2. Is this the correct way to setup virtualenv via poetry (please refer screenshot below) ? I followed what was mentioned in the Working with DEM source section of the guidelines. If required we'll troubleshoot this further on discord.
    image
    I also tried running the dem create DEV_ENV_NAME but was receiving error as shown in my earlier message (1st Screenshot).

  3. I have docker engine version 24.0.6 (PFB screenshot)
    image

@janosmurai
Copy link
Contributor

janosmurai commented Oct 23, 2023

Yes, my bad, I see it now that you were in the virtual env. You set it up the right way.
It looks like every dependency is correct. Could you please also try the simple comand dem --version.

@Kabiirk
Copy link
Contributor

Kabiirk commented Oct 24, 2023

Hi Janos,

dem --version works for me. I have messaged you on Discord to take things forward.

@Kabiirk
Copy link
Contributor

Kabiirk commented Oct 25, 2023

Hi all,

Done with coding this feature, the outputs are as follows:

  1. When there are no Hosts in the config_file
    image

  2. Where there are hosts available in the config_file
    image

  3. dem list-host --help also works
    image

Let me know if this works & I can make the PR or what are the changes, if any. (for e.g. dem add-host --help mentions that host['address'] should be an IP address & not a URL, w.r.t that, Let me know if address of Test4 of Output 2 is fine with you guy, or if that is a 'bug'.)

Best Regards,
Kabiir Krishna

@janosmurai
Copy link
Contributor

janosmurai commented Oct 26, 2023

The command usage LGTM. If your modification complies with the contribution guide, you can open the PR.
The dem add-host --help states that the address input parameter is IP or hostname of the host. The way you tested this command is okay.
In the future, a check will be implemented to inform the user that the input IP address or hostname of the dem add-host command points to a server capable of acting as a remote host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest Issue to help with during Hacktoberfest2023
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants