anancarv / python-artifactory

Typed interactions with the Jfrog Artifactory REST API
MIT License
55 stars 50 forks source link

Added artifact listing #123

Closed bdsoha closed 1 year ago

bdsoha commented 1 year ago

Description

Added a feature tp retrieve a recursive list of artifacts at a give path.

Closes: #94

Type of change

Please delete options that are not relevant.

How has it been tested ?

Once the general concept is approved, I will added automated unit-tests.

Checklist:

bdsoha commented 1 year ago

@anancarv Hey, any chance you can review this? Thanks in advance.

anancarv commented 1 year ago

@anancarv Hey, any chance you can review this? Thanks in advance.

Hey, yes I'll review it today 👍🏿

bdsoha commented 1 year ago

@anancarv Any updates, tips?

anancarv commented 1 year ago

@anancarv Any updates, tips?

@anancarv Any updates, tips?

hey, I have nothing to add to the changes proposed by @nymous. Once he gives his approval, the PR will be good to go

nymous commented 1 year ago

Since we agree with the feature as implemented, I think you can add the unit tests you were thinking about :wink:

bdsoha commented 1 year ago

@nymous There is somewhat of an issue to use Literal[True], because we are also relying on the built-in casting mechanism provided by pydantic. The value returned from the API is not the boolean value True, but rather a string value of "true".

When using the bool type. the string value is converted to an actual boolean. However, when using Literal, the value is rejected altogether.

One solution, would be to use @validator, which IMO is overkill. Another solution, is to join both the ArtifactListFolderResponse and ArtifactListFileResponse into a single object and not rely on the folder property being used as a discriminator.

nymous commented 1 year ago

Are you sure about the response from the API? I just tested on our Artifactory SaaS instance (version 7.46.7) and it has "real" booleans

{
  "uri": "https://<instance>.jfrog.io/artifactory/api/storage/<docker-registry>",
  "created": "2022-11-24T10:22:29.780Z",
  "files": [
    {
      "uri": "/.jfrog",
      "size": -1,
      "lastModified": "2022-10-17T10:50:44.639Z",
      "folder": true
    },
    {
      "uri": "/.jfrog/repository.catalog",
      "size": 197,
      "lastModified": "2022-11-15T10:26:43.051Z",
      "folder": false,
      "sha1": "40456bb6c5a001f40645920f653472f0b5c05ffa",
      "sha2": "6b7358e59df6f5953b8d83dccce2cce2f7a382d5d2f14cefeb68564485015cc4"
    }
  ]
}
bdsoha commented 1 year ago

@nymous Looks like you are correct, I was comparing to the documentation.

bdsoha commented 1 year ago

@anancarv @nymous Can you have a look at the changes?