Skip to content

Commit

Permalink
[AUD-1828] Remove routes from external documentation (#2856)
Browse files Browse the repository at this point in the history
* Fix flask plugin to not check route params for undocumented routes

* Don't document trending/version, recommended, and get latest track

* Don't document associated wallets, user challenges

* Undocument challenges routes

* Undocument metrics route

* Remove trending playlists with version
  • Loading branch information
rickyrombo committed Apr 7, 2022
1 parent cabab2e commit a5ea573
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,24 @@ def get(self):
pass
"""

doc_false_with_route_param_example = """
@ns.route(
\"/trending\",
defaults={\"version\": DEFAULT_TRENDING_VERSIONS[TrendingType.TRACKS].name},
strict_slashes=False,
doc={
"get": {
"id": \"Get Trending Tracks\",
"description": \"Gets the top 100 trending (most popular) tracks on Audius\",
}
},
)
@ns.route(\"/trending/<string:version>\", doc=False)
class Trending(Resource):
def get(self, version):
pass
"""


def _results(s: str):
tree = ast.parse(s)
Expand Down Expand Up @@ -275,6 +293,10 @@ def test_method_id_in_route():
assert not _results(method_id_in_route_doc_example)


def test_doc_false_with_route_param():
assert not _results(doc_false_with_route_param_example)


def test_plugin_format():
tree = ast.parse(non_route_param_example)
plugin = Plugin(tree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
find_item_to_move_above,
is_api_expect_decorator,
is_route_decorator,
is_route_decorator_documented,
parse_route_args,
)

Expand All @@ -25,21 +26,22 @@ def __init__(self, decorator_order=[], api_doc_keyword_order=[]):
def visit_ClassDef(self, node: ast.ClassDef):
route_args_dict: Dict[str, RouteArgEntry] = dict()
can_check_route_args = False
has_documented_route = False
has_documented_route = False # If there's at least one route that should be documented for this class
has_class_level_expect = False
methods_with_ids = []
for decorator in node.decorator_list:
if isinstance(decorator, ast.Call):
if is_route_decorator(decorator):
# @api.route() decorators
if is_route_decorator(decorator) and is_route_decorator_documented(
decorator
):
has_documented_route = True
if parse_route_args(decorator, route_args_dict):
can_check_route_args = True

(
is_route_documented,
methods_with_ids,
) = find_and_process_route_doc(decorator, route_args_dict)
if is_route_documented:
has_documented_route = True
methods_with_ids.extend(
find_and_process_route_doc(decorator, route_args_dict)
)
elif is_api_expect_decorator(decorator):
has_class_level_expect = True

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ def is_route_decorator(decorator: ast.Call):
route_parser_regex = re.compile(".*<[^:]*:?(.*)>.*")


def is_route_decorator_documented(route_decorator: ast.Call):
"""Checks if the given route decorator is marked doc=False"""
for keyword in route_decorator.keywords:
if keyword.arg == "doc":
if isinstance(keyword.value, ast.Constant):
if keyword.value.value == False:
return False
return True


def parse_route_args(route_decorator: ast.Call, route_args_dict):
"""Parses an @api.route()'s first arg to get the route arguments"""
can_check_route_args = False
Expand Down Expand Up @@ -82,7 +92,7 @@ def find_and_process_route_doc(
methods_with_ids = [
method for method in get_method_ids_from_route_doc(keyword.value)
]
return (True, methods_with_ids)
return methods_with_ids


def find_param_descriptions_in_route_doc(doc: ast.Dict):
Expand Down
6 changes: 3 additions & 3 deletions discovery-provider/src/api/v1/challenges.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
)


@ns.route(attest_route)
@ns.route(attest_route, doc=False)
class Attest(Resource):
@ns.doc(
id="Get Challenge Attestation",
Expand Down Expand Up @@ -114,7 +114,7 @@ def get(self, challenge_id: str):
)


@ns.route(undisbursed_route)
@ns.route(undisbursed_route, doc=False)
class GetUndisbursedChallenges(Resource):
@ns.doc(
id="""Get Undisbursed Challenges""",
Expand Down Expand Up @@ -159,7 +159,7 @@ def get(self):
)


@ns.route(create_sender_attest_route)
@ns.route(create_sender_attest_route, doc=False)
class CreateSenderAttestation(Resource):
@ns.doc(
id="""Get Create Sender Attestation""",
Expand Down
2 changes: 1 addition & 1 deletion discovery-provider/src/api/v1/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def get(self):
)


@ns.route("/app_name/trailing/<string:time_range>")
@ns.route("/app_name/trailing/<string:time_range>", doc=False)
@ns.expect(trailing_app_name_parser)
class TrailingAppNameMetrics(Resource):
@ns.doc(
Expand Down
12 changes: 1 addition & 11 deletions discovery-provider/src/api/v1/playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,17 +309,7 @@ def get(self, playlist_id):
}
},
)
@ns.route(
"/trending/<string:version>",
doc={
"get": {
"id": """Get Trending Playlists With Version""",
"description": """Gets trending playlists for a time period based on the given trending strategy version""",
"params": {"version": "The strategy version of trending to use"},
"responses": {200: "Success", 400: "Bad request", 500: "Server error"},
}
},
)
@ns.route("/trending/<string:version>", doc=False)
class TrendingPlaylists(Resource):
@record_metrics
@ns.expect(trending_playlist_parser)
Expand Down
33 changes: 4 additions & 29 deletions discovery-provider/src/api/v1/tracks.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,16 +426,7 @@ def get(self):
}
},
)
@ns.route(
"/trending/<string:version>",
doc={
"get": {
"id": """Get Trending Tracks With Version""",
"description": """Gets the top 100 trending (most popular) tracks on Audius using a given trending strategy version""",
"params": {"version": "The strategy version of trending to use"},
}
},
)
@ns.route("/trending/<string:version>", doc=False)
class Trending(Resource):
@record_metrics
@ns.expect(trending_parser)
Expand Down Expand Up @@ -565,25 +556,9 @@ def get(self, version):
"/recommended",
defaults={"version": DEFAULT_TRENDING_VERSIONS[TrendingType.TRACKS].name},
strict_slashes=False,
doc={
"get": {
"id": """Get Recommended Tracks""",
"description": """Get recommended tracks""",
"responses": {200: "Success", 400: "Bad request", 500: "Server error"},
}
},
)
@ns.route(
"/recommended/<string:version>",
doc={
"get": {
"id": """Get Recommended Tracks With Version""",
"description": """Get recommended tracks using the given trending strategy version""",
"params": {"version": "The strategy version of trending to use"},
"responses": {200: "Success", 400: "Bad request", 500: "Server error"},
}
},
doc=False,
)
@ns.route("/recommended/<string:version>", doc=False)
class RecommendedTrack(Resource):
@record_metrics
@ns.expect(recommended_track_parser)
Expand Down Expand Up @@ -1070,7 +1045,7 @@ def get(self):
return success_response(tracks)


@ns.route("/latest")
@ns.route("/latest", doc=False)
class LatestTrack(Resource):
@record_metrics
@ns.doc(
Expand Down
4 changes: 2 additions & 2 deletions discovery-provider/src/api/v1/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ def get(self):
@ns.deprecated
@ns.route(
"/associated_wallets",
doc={"description": "(Deprecated in favor of `/<user_id>/connected_wallets`)"},
doc=False,
)
class AssociatedWalletByUserId(Resource):
@ns.doc(
Expand Down Expand Up @@ -867,7 +867,7 @@ def get(self, replica_type):
)


@ns.route("/<string:id>/challenges")
@ns.route("/<string:id>/challenges", doc=False)
class GetChallenges(Resource):
@ns.doc(
id="""Get User Challenges""",
Expand Down

0 comments on commit a5ea573

Please sign in to comment.