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

Support container logs #54

Merged
merged 3 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions backend/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ProvidersEnum,
)
from backend.fetcher import (
ContainerProvider,
CoprProvider,
KojiProvider,
PackitProvider,
Expand Down Expand Up @@ -58,7 +59,7 @@
@app.exception_handler(RequestValidationError)
def _custom_http_exception_handler(
request: Request, exc: HTTPException | RequestValidationError | Exception
):
) -> JSONResponse:
if isinstance(exc, HTTPException):
status_code = exc.status_code
elif isinstance(exc, RequestValidationError):
Expand Down Expand Up @@ -103,7 +104,9 @@ def review(request: Request):

@app.get("/frontend/contribute/copr/{build_id}/{chroot}")
@app.get("/frontend/contribute/koji/{build_id}/{chroot}")
def get_build_logs_with_chroot(request: Request, build_id: int, chroot: str):
def get_build_logs_with_chroot(
request: Request, build_id: int, chroot: str
) -> ContributeResponseSchema:
provider_name = request.url.path.lstrip("/").split("/")[2]
prov_kls = CoprProvider if provider_name == ProvidersEnum.copr else KojiProvider
provider = prov_kls(build_id, chroot)
Expand Down Expand Up @@ -148,6 +151,18 @@ def get_build_logs_from_url(base64: str) -> ContributeResponseSchema:
)


@app.get("/frontend/contribute/container/{base64}")
def get_logs_from_container(base64: str) -> ContributeResponseSchema:
build_url = b64decode(base64).decode("utf-8")
provider = ContainerProvider(build_url)
return ContributeResponseSchema(
build_id=None,
build_id_title=BuildIdTitleEnum.container,
build_url=build_url,
logs=provider.fetch_logs(),
)


# TODO: no response checking here, it will be deleted anyway
@app.get("/frontend/contribute/debug")
def get_debug_build_logs():
Expand Down Expand Up @@ -176,7 +191,12 @@ def _store_data_for_providers(
feedback_input: FeedbackInputSchema, provider: ProvidersEnum, id_: int | str, *args
) -> OkResponse:
storator = Storator3000(provider, id_)
result_to_store = schema_inp_to_out(feedback_input)

if provider == ProvidersEnum.container:
result_to_store = schema_inp_to_out(feedback_input, is_with_spec=False)
else:
result_to_store = schema_inp_to_out(feedback_input)

storator.store(result_to_store)
if len(args) > 0:
rest = f"/{args[0]}"
Expand Down Expand Up @@ -215,6 +235,13 @@ def contribute_review_url(feedback_input: FeedbackInputSchema, url: str) -> OkRe
return _store_data_for_providers(feedback_input, ProvidersEnum.url, url)


@app.post("/frontend/contribute/container/{url}")
def contribute_review_container_logs(
feedback_input: FeedbackInputSchema, url: str
) -> OkResponse:
return _store_data_for_providers(feedback_input, ProvidersEnum.container, url)


@app.get("/frontend/review")
def frontend_review() -> FeedbackSchema:
if os.environ.get("ENV") == "production":
Expand Down
2 changes: 2 additions & 0 deletions backend/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class ProvidersEnum(StrEnum):
copr = "copr"
koji = "koji"
url = "url"
container = "container"
debug = "debug"


Expand All @@ -21,4 +22,5 @@ class BuildIdTitleEnum(StrEnum):
koji = "Koji build"
packit = "Packit build"
url = "URL"
container = "Container log"
debug = "Debug output"
53 changes: 43 additions & 10 deletions backend/fetcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ def fetch_logs(self) -> list[dict[str, str]]:
"""
...


class RPMProvider(Provider):
"""
Is able to provide spec file on top of the logs.
"""

@abstractmethod
def fetch_spec_file(self) -> dict[str, str]:
"""
Expand All @@ -72,7 +78,7 @@ def fetch_spec_file(self) -> dict[str, str]:
...


class CoprProvider(Provider):
class CoprProvider(RPMProvider):
copr_url = "https://copr.fedorainfracloud.org"

def __init__(self, build_id: int, chroot: str) -> None:
Expand Down Expand Up @@ -130,7 +136,7 @@ def fetch_spec_file(self) -> dict[str, str]:
return {"name": spec_name, "content": response.text}


class KojiProvider(Provider):
class KojiProvider(RPMProvider):
koji_url = "https://koji.fedoraproject.org"
logs_to_look_for = ["build.log", "root.log", "mock_output.log"]

Expand Down Expand Up @@ -269,7 +275,7 @@ def fetch_spec_file(self) -> dict[str, str]:
return spec_dict


class PackitProvider(Provider):
class PackitProvider(RPMProvider):
"""
The `packit_id` is hard to get. Open https://prod.packit.dev/api

Expand All @@ -289,17 +295,19 @@ def __init__(self, packit_id: int) -> None:
self.koji_url = f"{self.packit_api_url}/koji-builds/{self.packit_id}"

def _get_correct_provider(self) -> CoprProvider | KojiProvider:
build = requests.get(self.copr_url).json()
if "error" not in build:
resp = requests.get(self.copr_url)
if resp.ok:
build = resp.json()
return CoprProvider(build["build_id"], build["chroot"])

build = requests.get(self.koji_url).json()
task_id = build["build_id"]
if "error" in build:
resp = requests.get(self.koji_url)
if not resp.ok:
raise FetchError(
f"Couldn't find any build logs for Packit ID #{self.packit_id}."
)

build = resp.json()
task_id = build["task_id"]
koji_api_url = f"{KojiProvider.koji_url}/kojihub"
koji_client = koji.ClientSession(koji_api_url)
arch = koji_client.getTaskInfo(task_id, strict=True).get("arch")
Expand All @@ -317,7 +325,7 @@ def fetch_spec_file(self) -> dict[str, str]:
return self._get_correct_provider().fetch_spec_file()


class URLProvider(Provider):
class URLProvider(RPMProvider):
def __init__(self, url: str) -> None:
self.url = url

Expand All @@ -327,7 +335,7 @@ def fetch_logs(self) -> list[dict[str, str]]:
# also this will allow us to fetch spec files
response = requests.get(self.url)
response.raise_for_status()
if response.headers["Content-Type"] != "text/plain":
if "text/plain" not in response.headers["Content-Type"]:
raise FetchError(
"The URL must point to a raw text file. " f"This URL isn't: {self.url}"
)
Expand All @@ -345,6 +353,31 @@ def fetch_spec_file(self) -> dict[str, str]:
return {"name": "fake_spec_name.spec", "content": "fake spec file"}


class ContainerProvider(Provider):
"""
Fetching container logs only from URL for now
"""

def __init__(self, url: str) -> None:
self.url = url

@handle_errors
def fetch_logs(self) -> list[dict[str, str]]:
# TODO: c&p from url provider for now, integrate with containers better later on
response = requests.get(self.url)
response.raise_for_status()
if "text/plain" not in response.headers["Content-Type"]:
raise FetchError(
"The URL must point to a raw text file. " f"This URL isn't: {self.url}"
)
return [
{
"name": "Container log",
"content": response.text,
}
]


def fetch_debug_logs():
return [
{
Expand Down
56 changes: 41 additions & 15 deletions backend/schema.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
from typing import Optional

from pydantic import AnyUrl, BaseModel
from pydantic import AnyUrl, BaseModel, root_validator

from backend.constants import BuildIdTitleEnum


class LogSchema(BaseModel):
name: str
content: str
def _check_spec_container_are_exclusively_mutual(_, values):
spec_file = values.get("spec_file")
container_file = values.get("container_file")
if spec_file and container_file:
raise ValueError("You can't specify both spec file and container file")

return values

class SpecfileSchema(BaseModel):
# TODO: do we want to store spec_file separately
# in file or store content in one file?
# or the path means just its name?
# path: Path

class NameContentSchema(BaseModel):
# TODO: do we want to store spec_file and container_file separately
# in file or store content in one file? Or the path means just its name?
name: str
content: str

Expand All @@ -25,17 +27,25 @@ class ContributeResponseSchema(BaseModel):
fetched data (logs, spec, ...) and are needed for user to give a feedback why
build failed.
"""

build_id: Optional[int]
build_id_title: BuildIdTitleEnum
build_url: AnyUrl
logs: list[LogSchema]
spec_file: SpecfileSchema
logs: list[NameContentSchema]
spec_file: Optional[NameContentSchema] = None
container_file: Optional[NameContentSchema] = None

# validators
_normalize_spec_and_container_file = root_validator(pre=True, allow_reuse=True)(
_check_spec_container_are_exclusively_mutual
)


class SnippetSchema(BaseModel):
"""
Snippet for log, each log may have 0 - many snippets.
"""

# LINE_FROM:CHAR_FROM-LINE_TO:CHAR_TO
# log_part: constr(regex=r"^\d+:\d+-\d+:\d+$")
start_index: int
Expand All @@ -56,25 +66,33 @@ class SnippetSchema(BaseModel):
# return self._splitter(False)


class FeedbackLogSchema(LogSchema):
class FeedbackLogSchema(NameContentSchema):
"""
Feedback from user per individual log with user's comment.
"""

snippets: list[SnippetSchema]


class _WithoutLogsSchema(BaseModel):
username: Optional[str]
spec_file: SpecfileSchema
fail_reason: str
how_to_fix: str
spec_file: Optional[NameContentSchema] = None
container_file: Optional[NameContentSchema] = None

# validators
_normalize_spec_and_container_file = root_validator(pre=True, allow_reuse=True)(
_check_spec_container_are_exclusively_mutual
)


class FeedbackInputSchema(_WithoutLogsSchema):
"""
Contains data from users with reasons why build failed. It is sent from FE
and contains only inputs from user + spec and logs content.
"""

logs: list[FeedbackLogSchema]


Expand All @@ -83,18 +101,26 @@ class FeedbackSchema(_WithoutLogsSchema):
This schema is the final structure as we decided to store our data in json file
from users feedbacks.
"""

logs: dict[str, FeedbackLogSchema]


def schema_inp_to_out(inp: FeedbackInputSchema) -> FeedbackSchema:
def schema_inp_to_out(
inp: FeedbackInputSchema, is_with_spec: bool = True
) -> FeedbackSchema:
parsed_log_schema = {}
for log_schema in inp.logs:
parsed_log_schema[log_schema.name] = log_schema

if is_with_spec:
spec_or_container = {"spec_file": inp.spec_file}
else:
spec_or_container = {"container_file": inp.container_file}

return FeedbackSchema(
username=inp.username,
spec_file=inp.spec_file,
logs=parsed_log_schema,
fail_reason=inp.fail_reason,
how_to_fix=inp.how_to_fix,
**spec_or_container,
)
2 changes: 1 addition & 1 deletion backend/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def store(self, feedback_result: FeedbackSchema) -> None:
timestamp_seconds = int(datetime.now().timestamp())
file_name = self.build_dir / f"{timestamp_seconds}.json"
with open(file_name, "w") as fp:
json.dump(feedback_result.dict(), fp, indent=4)
json.dump(feedback_result.dict(exclude_unset=True), fp, indent=4)

@staticmethod
def _get_random_dir_from(dir_: Path) -> Path:
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/app/contribute.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
snippets
files
spec
container
error-description
error-title
backend-data
Expand Down Expand Up @@ -64,6 +65,7 @@
(reset! build-id-title (:build_id_title data))
(reset! build-url (:build_url data))
(reset! spec (:spec_file data))
(reset! container (:container_file data))

(reset!
files
Expand All @@ -85,7 +87,7 @@
[(instructions-item
(not-empty @files)

(if (= @build-id-title "URL")
(if (contains? #{"URL" "Container log"} @build-id-title)
[:<>
(str "We fetched logs from ")
[:a {:href @build-url} "this URL"]]
Expand Down
1 change: 1 addition & 0 deletions frontend/src/app/contribute_atoms.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
(def snippets (r/atom []))
(def files (r/atom nil))
(def spec (r/atom nil))
(def container (r/atom nil))

(def error-description (r/atom nil))
(def error-title (r/atom nil))
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/app/contribute_events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
snippets
fas
spec
container
files]]))


Expand Down Expand Up @@ -53,7 +54,8 @@
;; We can't use @files here because they contain highlight
;; spans, HTML escaping, etc.
(:logs @backend-data))
:spec_file @spec}]
:spec_file @spec
:container_file @container}]
(reset! status "submitting")
(-> (fetch/post url {:accept :json :content-type :json :body body})
(.then (fn [resp] (-> resp :body (js->clj :keywordize-keys true))))
Expand Down
Loading