-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat: add Artifactory Query Language integration #72
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
"Artifactory queries" | ||
from typing import List, Dict, Optional, Union, Any | ||
from enum import Enum | ||
|
||
from pydantic import BaseModel | ||
|
||
|
||
class SortTypesEnum(str, Enum): | ||
"Order of query results" | ||
asc = "$asc" | ||
desc = "$desc" | ||
|
||
|
||
class DomainQueryEnum(str, Enum): | ||
"Artifactory domain objects to be queried" | ||
items = "items" | ||
builds = "builds" | ||
entries = "entries" | ||
|
||
|
||
class Aql(BaseModel): | ||
"Artifactory Query Language" | ||
domain: DomainQueryEnum = DomainQueryEnum.items | ||
find: Optional[Dict[str, Union[str, List[Dict[str, Any]], Dict[str, str]]]] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if you could make a self-referential type here...
But after thinking about it I'm not sure you could extract such a type with forward references, because it's not a Pydantic model but a union of many types... AqlFindCriterion = Dict[str, Union[str, Dict[str, str], List["AqlFindCriterion"]]] |
||
include: Optional[List[str]] | ||
sort: Optional[Dict[SortTypesEnum, List[str]]] = None | ||
offset: Optional[int] = None | ||
limit: Optional[int] = None |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,7 @@ | ||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||
Definition of all artifactory objects. | ||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||
import json | ||||||||||||||||||||||||||
import warnings | ||||||||||||||||||||||||||
import logging | ||||||||||||||||||||||||||
import os | ||||||||||||||||||||||||||
|
@@ -24,6 +25,7 @@ | |||||||||||||||||||||||||
PermissionNotFoundException, | ||||||||||||||||||||||||||
InvalidTokenDataException, | ||||||||||||||||||||||||||
PropertyNotFoundException, | ||||||||||||||||||||||||||
AqlException, | ||||||||||||||||||||||||||
ArtifactNotFoundException, | ||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
@@ -48,6 +50,7 @@ | |||||||||||||||||||||||||
User, | ||||||||||||||||||||||||||
Permission, | ||||||||||||||||||||||||||
SimplePermission, | ||||||||||||||||||||||||||
Aql, | ||||||||||||||||||||||||||
ArtifactPropertiesResponse, | ||||||||||||||||||||||||||
ArtifactStatsResponse, | ||||||||||||||||||||||||||
ArtifactInfoResponse, | ||||||||||||||||||||||||||
|
@@ -73,6 +76,7 @@ def __init__( | |||||||||||||||||||||||||
self.repositories = ArtifactoryRepository(self.artifactory) | ||||||||||||||||||||||||||
self.artifacts = ArtifactoryArtifact(self.artifactory) | ||||||||||||||||||||||||||
self.permissions = ArtifactoryPermission(self.artifactory) | ||||||||||||||||||||||||||
self.aql = ArtifactoryAql(self.artifactory) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
class ArtifactoryObject: | ||||||||||||||||||||||||||
|
@@ -950,3 +954,53 @@ def delete(self, artifact_path: str) -> None: | |||||||||||||||||||||||||
artifact_path = artifact_path.lstrip("/") | ||||||||||||||||||||||||||
self._delete(f"{artifact_path}") | ||||||||||||||||||||||||||
logger.debug("Artifact %s successfully deleted", artifact_path) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def create_aql_query(aql_object: Aql): | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this function could become a method in the aql = Aql(<params>)
aql.to_query() # items.find().include()... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I think this method could benefit from being more unit tested, to make sure we generate the correct query whatever the combination of parameters we give. I can write those if you want 😉 |
||||||||||||||||||||||||||
"Create Artifactory query" | ||||||||||||||||||||||||||
aql_query_text = f"{aql_object.domain}.find" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if aql_object.find: | ||||||||||||||||||||||||||
aql_query_text += f"({json.dumps(aql_object.find)})" | ||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||
aql_query_text += "()" | ||||||||||||||||||||||||||
Comment on lines
+961
to
+966
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To keep it consistent with the other parts of the function (and simpler to understand), maybe move the
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if aql_object.include: | ||||||||||||||||||||||||||
format_include = ( | ||||||||||||||||||||||||||
json.dumps(aql_object.include).replace("[", "").replace("]", "") | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about this, what would happen if there is a |
||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
aql_query_text += f".include({format_include})" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if aql_object.sort: | ||||||||||||||||||||||||||
sort_key = list(aql_object.sort.keys())[0] | ||||||||||||||||||||||||||
sort_value = json.dumps(aql_object.sort[sort_key]) | ||||||||||||||||||||||||||
aql_query_text += f'.sort({{"{sort_key.value}": {sort_value}}})' | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if aql_object.offset: | ||||||||||||||||||||||||||
aql_query_text += f".offset({aql_object.offset})" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if aql_object.limit: | ||||||||||||||||||||||||||
aql_query_text += f".limit({aql_object.limit})" | ||||||||||||||||||||||||||
Comment on lines
+979
to
+983
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Be careful with this sort of comparison, |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return aql_query_text | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
class ArtifactoryAql(ArtifactoryObject): | ||||||||||||||||||||||||||
"Artifactory Query Language support" | ||||||||||||||||||||||||||
_uri = "search/aql" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def query(self, aql_object: Aql) -> List[Dict[str, Union[str, List]]]: | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But then I wondered about the API we expose... Should the user create an Aql object, and pass it to the method, or should the method take parameters like |
||||||||||||||||||||||||||
"Send Artifactory query" | ||||||||||||||||||||||||||
aql_query = create_aql_query(aql_object) | ||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||
response = self._post(f"api/{self._uri}", data=aql_query) | ||||||||||||||||||||||||||
response_content: List[Dict[str, Union[str, List]]] = response.json()[ | ||||||||||||||||||||||||||
"results" | ||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||
Comment on lines
+997
to
+999
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should parse the response here, with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, actually should we return only the |
||||||||||||||||||||||||||
logging.debug("Successful query") | ||||||||||||||||||||||||||
return response_content | ||||||||||||||||||||||||||
except requests.exceptions.HTTPError as error: | ||||||||||||||||||||||||||
raise AqlException( | ||||||||||||||||||||||||||
"Bad Aql Query: please check your parameters." | ||||||||||||||||||||||||||
"Doc: https://www.jfrog.com/confluence/display/RTF/Artifactory+Query+Language" | ||||||||||||||||||||||||||
) from error |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,55 @@ | ||||||
import pytest | ||||||
import responses | ||||||
|
||||||
from pyartifactory import ArtifactoryAql | ||||||
from pyartifactory.exception import AqlException | ||||||
from pyartifactory.models import AuthModel, Aql | ||||||
|
||||||
URL = "http://localhost:8080/artifactory" | ||||||
AUTH = ("user", "password_or_apiKey") | ||||||
AQL_RESPONSE = { | ||||||
"results": [ | ||||||
{ | ||||||
"repo": "libs-release-local", | ||||||
"path": "org/jfrog/artifactory", | ||||||
"name": "artifactory.war", | ||||||
"type": "item type", | ||||||
"size": "75500000", | ||||||
"created": "2015-01-01T10:10;10", | ||||||
"created_by": "Jfrog", | ||||||
"modified": "2015-01-01T10:10;10", | ||||||
"modified_by": "Jfrog", | ||||||
"updated": "2015-01-01T10:10;10", | ||||||
} | ||||||
], | ||||||
"range": {"start_pos": 0, "end_pos": 1, "total": 1}, | ||||||
} | ||||||
|
||||||
|
||||||
@responses.activate | ||||||
def test_aql_success(): | ||||||
responses.add( | ||||||
responses.POST, f"{URL}/api/search/aql", json=AQL_RESPONSE, status=200 | ||||||
) | ||||||
|
||||||
artifactory_aql = ArtifactoryAql(AuthModel(url=URL, auth=AUTH)) | ||||||
aql_obj = Aql(**{"find": {"repo": {"$eq": "libs-release-local"}}}) | ||||||
result = artifactory_aql.query(aql_obj) | ||||||
assert result == AQL_RESPONSE["results"] | ||||||
|
||||||
|
||||||
@responses.activate | ||||||
def test_aql_fail_baq_query(): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wooops, that should read ^^
Suggested change
|
||||||
responses.add( | ||||||
responses.POST, f"{URL}/api/search/aql", json=AQL_RESPONSE, status=400 | ||||||
) | ||||||
|
||||||
artifactory_aql = ArtifactoryAql(AuthModel(url=URL, auth=AUTH)) | ||||||
aql_obj = Aql( | ||||||
include=["artifact", "artifact.module", "artifact.module.build"], | ||||||
sort={"$asc": ["remote_downloaded"]}, | ||||||
limit=100, | ||||||
) | ||||||
|
||||||
with pytest.raises(AqlException): | ||||||
artifactory_aql.query(aql_obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly this part of the docs
Artifactory actually supports more than just the "items, builds or entries" that they announce. And even more, you cannot just use
entries.find()
(or at least on our instance it returns "Failedtoparsequery").FYI I tested all the primary domains they advertise in the quoted docs on our instance, and none of them fail (we have no results for releases and build.promotions but that may be because we never used the feature)