207 adding creation of an instance with payg #208

Antonyjin · 2024-05-22T13:22:10Z

…om the CLI The user don't have the possibility to create an instance using a CRN with PAYG by passing by the CLI Solution: Adding and ask if the user wants to create a PAYG instance. For now, we only ask if the user wants to create a PAYG instance. If yes, it shows a list of every compatible CRN. It takes some time, about 5 minutes to fetch the data. To do: Show the available ressources. Create the instance with PAYG Modify the instance message

While fetching compatible nodes, there is only a loading bar with a pourcentage. And then only at the end it displays the whole tab with every CRN hashes Solution: Display the tab within the fetch of nodes That way user have a visibiity on the fetch process and it's more visual

The hash given for the payg crn was not checked. Solution: Adding an error handling to check if the hash given by the user is a correct and compatible hash from the fetched list

Tab didn't show enough info about the CRN such as the console: (RAM, HDD, Version ...) Solution: Replace old info by these ones

Code did not checked if the payment was none in the case where it's not PAYG instance Solution: Default setting payment as None to pass in the message if it's not PAYG

Getting information and dislaying a live tab took too long Solution: Adding timeout and queue system for gain some time on the execution.

hoh

Nice start.

I added comments to improve the structure of the code.

Can you add type annotations and check the output of mypy and ruff check on the new code ?

src/aleph_client/commands/utils.py

hoh · 2024-05-29T23:11:52Z

src/aleph_client/commands/instance.py

@@ -178,6 +199,9 @@ def validate_ssh_pubkey_file(file: Union[str, Path]) -> Path:
    async with AuthenticatedAlephHttpClient(
        account=account, api_server=sdk_settings.API_HOST
    ) as client:
+        payment = None


Can you add a typing annotation ?

src/aleph_client/commands/utils.py

Originally the fetch of the version is about parsing an HTML text which was not the best thing to do Solution: use the last server header in the response instead to get the version of the CRN

I checked the request only with the status 200 which is not enough Solution: Using raise_for_status()

Some varibles don't have types Solution: Adding typing notation to them

Some of the request returned an error which wasn't checked This result to the end of execution of the script Solution: Adding a check about Exception and add the possibility to debug inside the logger

To stop it, we add 'None' at the end of the queue. But this could be more specific and clear in the code Solution: Creating a local variable which clearly tells that it is the end of the queuing process

src/aleph_client/commands/utils.py

Basically, we added a list of str inside the queue and then adding them inside the Live tab but this is unclear and could cause some misunderstanding from the users Solution: Creating a structure and pass this inside the queue The structure contains a field for MachineUsage and more for the required string such as the name, version, address, etc ... This permits to exactly know what is inside the queue and retrieve more easily the different information about it

Solution: Checking every error raised by my py and fix them

Solution: Fix them one by one

This documentation inside the functions is needed to a better understanding of how they works Solution: Adding docstring to each function

If the fetch of the version wasn't found (Timeout, connectionError, etc ...) There was a hardcoded string that replaced the version field. But this wasn't the best option. Solution: Return Optionnal[str] instead for being able to return a None value

Changed import paths to use explicit imports from specific modules within aleph_message.models, fixing incorrect or missing import paths. Solution: Specify the whole path for imports that wasn't well written

Solution: Changed the way hypervisor is selected to ensure type safety and consistency with HypervisorType enum. Previously, the hypervisor selection was done using Prompt.ask which returns a string. Now, the selected string is used to fetch the corresponding HypervisorType from the HypervisorType enum dictionary.

This could be simplified by calling get_message() function Solution: Instead of retrieving all messages and then selecting the program message from the list, this commit refactors the `unpersist` function to directly fetch the program message using the `get_message` method. This change simplifies the logic by ensuring that only the relevant program message is retrieved and processed.

hoh · 2024-06-25T15:30:41Z

Replaced with #216

Antonyjin added 3 commits May 21, 2024 15:10

Feat: CRN hash was not checked

15c4513

The hash given for the payg crn was not checked. Solution: Adding an error handling to check if the hash given by the user is a correct and compatible hash from the fetched list

Antonyjin requested a review from nesitor May 22, 2024 13:22

Antonyjin self-assigned this May 22, 2024

Antonyjin linked an issue May 22, 2024 that may be closed by this pull request

Adding creation of an instance with PAYG #207

Closed

Antonyjin added 3 commits May 24, 2024 15:09

Fix: Tab don't show the right info for the crn

eb18516

Tab didn't show enough info about the CRN such as the console: (RAM, HDD, Version ...) Solution: Replace old info by these ones

Fix: Error about payment in the message if it's not PAYG

d9cb7ba

Code did not checked if the payment was none in the case where it's not PAYG instance Solution: Default setting payment as None to pass in the message if it's not PAYG

Fix: The fetch CRN process was too long

bc0ec46

Getting information and dislaying a live tab took too long Solution: Adding timeout and queue system for gain some time on the execution.

Antonyjin requested a review from hoh May 29, 2024 09:59

hoh reviewed May 29, 2024

View reviewed changes

Antonyjin added 5 commits May 30, 2024 17:00

Fix: Using the last server header for getting the CRN version

43abce3

Originally the fetch of the version is about parsing an HTML text which was not the best thing to do Solution: use the last server header in the response instead to get the version of the CRN

Fix: Http check was not perfect

b0064df

I checked the request only with the status 200 which is not enough Solution: Using raise_for_status()

Fix: Missing typing notation

6215eaf

Some varibles don't have types Solution: Adding typing notation to them

Fix: Script crash because of unhandled errors

60a9a9c

Some of the request returned an error which wasn't checked This result to the end of execution of the script Solution: Adding a check about Exception and add the possibility to debug inside the logger

Fix: The way to stop the queue process was not clear enough

faf6351

To stop it, we add 'None' at the end of the queue. But this could be more specific and clear in the code Solution: Creating a local variable which clearly tells that it is the end of the queuing process

hoh reviewed Jun 3, 2024

View reviewed changes

Antonyjin and others added 13 commits June 4, 2024 18:17

Fix: mypy raised a lot of errors

7db3cf8

Solution: Checking every error raised by my py and fix them

Fix: Mypy raised some error in instance.py

816fb6e

Solution: Fix them one by one

Fix: Ruff raised some errors about placeholders with f-string

f5a86d5

Fix: Missing docstrings in the new functions

33fe36a

This documentation inside the functions is needed to a better understanding of how they works Solution: Adding docstring to each function

Fix: Missing type annotation inside utils.py

83c576e

Fix: Refactor import statements in aleph_client/utils.py

33fdc09

Changed import paths to use explicit imports from specific modules within aleph_message.models, fixing incorrect or missing import paths. Solution: Specify the whole path for imports that wasn't well written

Fix: Refactor import statements in aleph_client/utils.py

d229436

Changed import paths to use explicit imports from specific modules within aleph_message.models, fixing incorrect or missing import paths. Solution: Specify the whole path for imports that wasn't well written

Fix: Library psutil was not useful in aleph-client

267f16c

Cleanup: black and isort

9f25a55

hoh added 5 commits June 7, 2024 14:03

Refactor: Move models in distinct file

94f5af4

Cleanup: Move imports and logger

6179601

Refactor: Large refactoring in pair programming

91545cd

Add tests \o/

1f8f0cd

Fix type checking

8eda387

hoh closed this Jun 25, 2024

Psycojoker deleted the 207-adding-creation-of-an-instance-with-payg branch July 24, 2024 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

207 adding creation of an instance with payg #208

207 adding creation of an instance with payg #208

Antonyjin commented May 22, 2024 •

edited

Loading

hoh left a comment

hoh May 29, 2024

hoh commented Jun 25, 2024

207 adding creation of an instance with payg #208

207 adding creation of an instance with payg #208

Conversation

Antonyjin commented May 22, 2024 • edited Loading

hoh left a comment

Choose a reason for hiding this comment

hoh May 29, 2024

Choose a reason for hiding this comment

hoh commented Jun 25, 2024

Antonyjin commented May 22, 2024 •

edited

Loading