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

Major sdk update #62

Merged
merged 60 commits into from
Sep 26, 2024
Merged

Major sdk update #62

merged 60 commits into from
Sep 26, 2024

Conversation

Arondondon
Copy link
Collaborator

@Arondondon Arondondon commented Aug 5, 2024

Update includes the following:

  1. Removing CLI dependencies. Now SDK does not depend on CLI in any way and works separately. In addition, as a result of this, the logic of some methods in SDK has changed, as well as the project structure - the client_lib_generator, utils, ipfs_utils, config and service_metadata modules have been added.
  2. Documentation. Complete technical documentation has been written (project directory "docs"). The README.md file has also been updated, now it contains a complete description of all the functionality of the Python SDK.
  3. Examples. There are two examples of using Python SDK to create custom applications. Among them, one is small - a calculator based on the SIngularityNET platform service, and the second is a full-fledged console application that uses all the functionality of the SDK. For both examples, .md files with tutorials on creating these examples are written. (Project directory "examples").

@Arondondon Arondondon changed the title Development Big update Sep 10, 2024
Copy link
Member

@kiruxaspb kiruxaspb left a comment

Choose a reason for hiding this comment

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

Docs approved

Copy link
Member

@kiruxaspb kiruxaspb left a comment

Choose a reason for hiding this comment

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

Approved updated docs

mirel-dev and others added 2 commits September 20, 2024 16:40
…d only in code and is not stored in .snet folder.
Got rid of local storage of client configuration.
Copy link
Member

@kiruxaspb kiruxaspb left a comment

Choose a reason for hiding this comment

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

Update docs for config update

@kiruxaspb
Copy link
Member

kiruxaspb commented Sep 23, 2024

@Necr0x0Der Could your please review this one PR?

Here we have updated the moment and now private keys are not stored in idenitity.

@Necr0x0Der
Copy link

@Necr0x0Der Could your please review this one PR?

Here we have updated the moment and now private keys are not stored in idenitity.

Hmmm... The PR is quite large for a comprehensive review for me atm. What I see is that a lot of code was moved from cli to sdk, which is good. Config has become more compact and private keys are not stored, which are also good. This can be easily adopted in sdk in metta.
I'm not sure why there are so many proto files and pre-build pb2_grpc files in resources? Do they describe a standard API for daemons and are not stored on ipfs?
In any case, I don't see issues with changes on the client side from a quick glance.

@kiruxaspb
Copy link
Member

I'm not sure why there are so many proto files and pre-build pb2_grpc files in resources? Do they describe a standard API for daemons and are not stored on ipfs? In any case, I don't see issues with changes on the client side from a quick glance.

Thank you for review, @Necr0x0Der.

Yes, proto files stored in resources describe fixed API for daemons and IPFS downloads checks.
These are service protos, and I think it's more efficient to store them immediately in the package than to add additional downloading from IPFS, compiling and custom importing of these proto files in snet.sdk codebase.

We will add the issue of moving service protos to decentralized storage in our backlog.

@DaddyWesker
Copy link

DaddyWesker commented Sep 25, 2024

Initializes the m dict with empty of default values.

Probably there should be 'or' not 'of'? It is in https://github.com/Arondondon/snet-sdk-python/blob/development/docs/metadata_provider/service_metadata.md file

If the endpoint is not valid of if the group does not exist

Same here?

@DaddyWesker
Copy link

DaddyWesker commented Sep 26, 2024

Code from calculator example.

def parse_expression(expression):
    elements = list(expression.split())
    if len(elements) != 3:
        raise Exception(f"Invalid expression '{expression}'. Three items required.")

    a = int(elements[0])
    b = int(elements[2])
    if elements[1] not in ["+", "-", "*", "/"]:
        raise Exception(f"Invalid expression '{expression}'. Operation must be '+' or '-' or '*' or '/'")
    elif not isinstance(a, (float, int)) or not isinstance(b, (float, int)):
        raise Exception(f"Invalid expression '{expression}'. Operands must be integers or floating point numbers.")
    op = elements[1]

    return a, b, op

Maybe I'm wrong, but elif not isinstance(a, (float, int)) or not isinstance(b, (float, int)): is useless currently since:

 a = int(elements[0])
 b = int(elements[2])

So, a and b will always be ints. Moreover, if user inputs float number it will be converted to int. Or I am missing something?

@Arondondon
Copy link
Collaborator Author

Thank you for review, @DaddyWesker.
You were right about the issues in service_metadata.md and calculator.py. Currently, they have been fixed.

Copy link
Member

@kiruxaspb kiruxaspb left a comment

Choose a reason for hiding this comment

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

Approved

@kiruxaspb kiruxaspb changed the title Big update Major sdk update Sep 26, 2024
@kiruxaspb kiruxaspb merged commit de2a6ed into singnet:development Sep 26, 2024
1 check failed
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.

5 participants