Skip to content

Fix crash in /requests/report/wait by removing missing requestReports #3564

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.

Sign up for GitHub

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

Open
iGufrankhan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix crash in /requests/report/wait by removing missing requestReports #3564

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions augur/api/view/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import logging
import re

from augur.api.view.init import report_requests
from flask import flash, current_app, jsonify, redirect, request, url_for

Check warning on line 5 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
W0611: Unused current_app imported from flask (unused-import)

Raw Output:
augur/api/view/api.py:5:0: W0611: Unused current_app imported from flask (unused-import)
from flask_login import current_user, login_required

from augur.application.db.models import Repo, RepoGroup, UserGroup, UserRepo

Check warning on line 8 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
W0611: Unused RepoGroup imported from augur.application.db.models (unused-import)

Raw Output:
augur/api/view/api.py:8:0: W0611: Unused RepoGroup imported from augur.application.db.models (unused-import)
from augur.application.db.session import DatabaseSession

Check warning on line 9 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
W0611: Unused DatabaseSession imported from augur.application.db.session (unused-import)

Raw Output:
augur/api/view/api.py:9:0: W0611: Unused DatabaseSession imported from augur.application.db.session (unused-import)
from augur.tasks.frontend import (
add_github_orgs_and_repos,
add_gitlab_repos,
Expand All @@ -21,23 +22,23 @@
def cache(file=None):
if file is None:
return redirect(url_for('static', filename="cache"))
return redirect(url_for('static', filename="cache/" + toCacheFilename(file, False)))

Check warning on line 25 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'toCacheFilename' (undefined-variable)

Raw Output:
augur/api/view/api.py:25:58: E0602: Undefined variable 'toCacheFilename' (undefined-variable)


def add_existing_org_to_group(session, user_id, group_name, rg_id):

logger.info("Adding existing org to group")

Check warning on line 30 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'logger' (undefined-variable)

Raw Output:
augur/api/view/api.py:30:4: E0602: Undefined variable 'logger' (undefined-variable)

group_id = UserGroup.convert_group_name_to_id(session, user_id, group_name)
if group_id is None:
return False

repos = session.query(Repo).filter(Repo.repo_group_id == rg_id).all()
logger.info("Length of repos in org: " + str(len(repos)))

Check warning on line 37 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'logger' (undefined-variable)

Raw Output:
augur/api/view/api.py:37:4: E0602: Undefined variable 'logger' (undefined-variable)
for repo in repos:
result = UserRepo.insert(session, repo.repo_id, group_id)
if not result:
logger.info("Failed to add repo to group")

Check warning on line 41 in augur/api/view/api.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'logger' (undefined-variable)

Raw Output:
augur/api/view/api.py:41:12: E0602: Undefined variable 'logger' (undefined-variable)



Expand Down Expand Up @@ -234,5 +235,4 @@
"""
@app.route('/requests/report/wait/<id>')
def wait_for_report_request(id):
requestReports(id)
return jsonify(report_requests[id])
Comment on lines -237 to -238
Copy link
Contributor

@guptapratykshh guptapratykshh Jan 12, 2026

Choose a reason for hiding this comment

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

can be updated like this -

    if id in report_requests:
        return jsonify(report_requests[id])
    
    return jsonify({
        "error": "Report request not found",
        "report_id": id
    }), 404

Copy link
Contributor Author

@iGufrankhan iGufrankhan Jan 12, 2026

Choose a reason for hiding this comment

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

thank for suggestion!That makes sense if report generation is still supported.
however as far i know that this he report generation pipeline(like requestReport) has been remove entrity
nothing populate report_requests
that why i changed minimal change so that fromted not get break
Let’s wait for @MoralCode to confirm, since I believe they were involved in removing the report pipeline.

return jsonify(report_requests.get(id, {}))
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Returning an empty dictionary when the report ID is not found may not be appropriate API design. Clients cannot distinguish between a missing report and an actual empty report result. Consider returning a 404 status code or an error response when the report ID doesn't exist in report_requests, which would provide clearer feedback to API consumers.

Suggested change
return jsonify(report_requests.get(id, {}))
if id not in report_requests:
return jsonify({
"error": "Report request not found",
"report_id": id
}), 404
return jsonify(report_requests[id])

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@MoralCode MoralCode Jan 11, 2026

Choose a reason for hiding this comment

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

If this value doesnt exist, doesnt that mean we are always going to be returning an empty dict?

I wonder if its better to completely remove the endpoint in that case if all its ever going to do is return an empty dict

Copy link
Contributor Author

@iGufrankhan iGufrankhan Jan 12, 2026

Choose a reason for hiding this comment

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

@MoralCode at first i think so
that if it gone always return empty then why not completely remove this endpoint but

The route already exists and is being called by the UI
Removing it now could break the UI or other code that still expects it
I didn’t remove the endpoint in this PR because it may still be referenced by the UI , and I wanted to keep the fix minimal and safe
i agree for
removing is the best option lets me check if remove this endpoint
code is not gone break
let me first verify it

Copy link
Contributor Author

@iGufrankhan iGufrankhan Jan 12, 2026

Choose a reason for hiding this comment

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

MainProcess] secondary:7e4c9aa35c4348ea9d2282425d939051@c6c439d5bd54 ready.
augur-1 | 2026-01-12 02:39:27 c6c439d5bd54 augur.api.server[121] ERROR Exception on /repos/views/repo/1 [GET]
augur-1 | Traceback (most recent call last):
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 2073, in wsgi_app
augur-1 | response = self.full_dispatch_request()
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 1518, in full_dispatch_request
augur-1 | rv = self.handle_user_exception(e)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask_cors/extension.py", line 178, in wrapped_function
augur-1 | return cors_after_request(app.make_response(f(*args, **kwargs)))
augur-1 | ^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 1516, in full_dispatch_request
augur-1 | rv = self.dispatch_request()
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 1502, in dispatch_request
augur-1 | return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/augur/api/view/routes.py", line 226, in repo_repo_view
augur-1 | return render_module("repo-info", title="Repo", repo=repo, repo_id=id)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/augur/api/view/utils.py", line 224, in render_module
augur-1 | return render_template('index.j2', **args)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/templating.py", line 147, in render_template
augur-1 | return _render(
augur-1 | ^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/templating.py", line 128, in _render
augur-1 | rv = template.render(context)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/jinja2/environment.py", line 1291, in render
augur-1 | self.environment.handle_exception()
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/jinja2/environment.py", line 925, in handle_exception
augur-1 | raise rewrite_traceback_stack(source=source)
augur-1 | File "/augur/augur/templates/index.j2", line 44, in top-level template code
augur-1 | {% include '%s.j2' % body ignore missing %}
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/augur/templates/repo-info.j2", line 23, in top-level template code
augur-1 | $.getJSON("{{ url_for('wait_for_report_request', id=repo_id) }}", function(image_data) {
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/helpers.py", line 338, in url_for
augur-1 | return appctx.app.handle_url_build_error(error, endpoint, values)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/helpers.py", line 325, in url_for
augur-1 | rv = url_adapter.build(
augur-1 | ^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/werkzeug/routing.py", line 2315, in build
augur-1 | raise BuildError(endpoint, values, method, self)
augur-1 | werkzeug.routing.BuildError: Could not build url for endpoint 'wait_for_report_request' with values ['id']. Did you mean 'repo_pull_requests_new' instead?
augur-keyman-1 | 2026-01-12 02:39:49,275 - KeyOrchestrator - INFO - ACK; for: 1
augur-keyman-1 | 2026-01-12 02:39:49,276 - KeyOrchestrator - INFO - ACK; for: 1

v View in Docker Desktop o View Config w Enable Watch

Copy link
Contributor Author

@iGufrankhan iGufrankhan Jan 12, 2026

Choose a reason for hiding this comment

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

see man
if remove the backend endpoint but
the frontend call this
flask give error if it is not present

so what do you think

Copy link
Contributor

@guptapratykshh guptapratykshh Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I recommend updating endpoint logic to return 404 error when report ID is not found.

Loading