Fix: Swagger UI for indexer is a stub (#5051, #5228) #5160

nadove-ucsc · 2023-04-26T00:05:15Z

Connected issues: #5051 #5228

Checklist

Author

PR is a draft
Target branch is develop
Name of PR branch matches issues/<GitHub handle of author>/<issue#>-<slug>
PR title references all connected issues
PR title matches¹ that of a connected issue _{or comment in PR explains why they're different}
For each connected issue, there is at least one commit whose title references that issue
PR is connected to all connected issues via ZenHub
PR description links to connected issues
Added partial label to PR _{or this PR completely resolves all connected issues}

¹ when the issue title describes a problem, the corresponding PR
title is Fix: followed by the issue title

Author (reindex, API changes)

Added r tag to commit title _{or this PR does not require reindexing}
Added reindex label to PR _{or this PR does not require reindexing}
Added a (compatible changes) or A (incompatible ones) tag to commit title _{or this PR does not modify the Azul service API}
Added API label to connected issues _{or this PR does not modify the Azul service API}

Author (chains)

This PR is blocked by previous PR in the chain _{or this PR is not chained to another PR}
Added base label to the blocking PR _{or this PR is not chained to another PR}
Added chained label to this PR _{or this PR is not chained to another PR}

Author (upgrading)

Documented upgrading of deployments in UPGRADING.rst _{or this PR does not require upgrading}
Added u tag to commit title _{or this PR does not require upgrading}
Added upgrade label to PR _{or this PR does not require upgrading}

Author (operator tasks)

Added checklist items for additional operator tasks _{or this PR does not require additional tasks}

Author (hotfixes)

Added F tag to main commit title _{or this PR does not include permanent fix for a temporary hotfix}
Reverted the temporary hotfixes for any connected issues _{or the prod branch has no temporary hotfixes for any connected issues}

Author (before every review)

Rebased PR branch on develop, squashed old fixups
Ran make requirements_update _{or this PR does not touch requirements*.txt, common.mk, Makefile and Dockerfile}
Added R tag to commit title _{or this PR does not touch requirements*.txt}
Added reqs label to PR _{or this PR does not touch requirements*.txt}
make integration_test passes in personal deployment _{or this PR does not touch functionality that could break the IT}

Peer reviewer (after requesting changes)

Uncheck the Author (before every review) checklists.

Peer reviewer (after approval)

PR is not a draft
Ticket is in Review requested column
Requested review from primary reviewer
Assigned PR to primary reviewer

Primary reviewer (after requesting changes)

Uncheck the before every review checklists. Update the N reviews label.

Primary reviewer (after approval)

Actually approved the PR
Labeled connected issues as demo or no demo
Commented on connected issues about demo expectations _{or all connected issues are labeled no demo}
Decided if PR can be labeled no sandbox
PR title is appropriate as title of merge commit
N reviews label is accurate
Moved ticket to Approved column
Assigned PR to current operator

Operator (before pushing merge the commit)

Operator (after pushing the merge commit)

¹ When pushing the merge commit is skipped due to the PR being
labelled no sandbox, the next build triggered by a PR whose merge commit is
pushed determines this checklist item.

Operator (reindex)

Deleted unreferenced indices in dev _{or this PR does not remove catalogs or otherwise causes unreferenced indices}
Deleted unreferenced indices in anvildev _{or this PR does not remove catalogs or otherwise causes unreferenced indices}
Started reindex in dev _{or this PR does not require reindexing}
Started reindex in anvildev _{or this PR does not require reindexing}
Checked for and triaged indexing failures in dev _{or this PR does not require reindexing}
Checked for and triaged indexing failures in anvildev _{or this PR does not require reindexing}
Emptied fail queues in dev deployment _{or this PR does not require reindexing}
Emptied fail queues in anvildev deployment _{or this PR does not require reindexing}

Operator

Unassigned PR

Shorthand for review comments

L line is too long
W line wrapping is wrong
Q bad quotes
F other formatting problem

coveralls · 2023-04-26T00:28:53Z

Coverage: 84.402% (+0.03%) from 84.373% when pulling 874f464 on issues/nadove-ucsc/5051-swagger-ui-indexer-stub into 75a01ee on develop.

codecov · 2023-04-26T00:31:55Z

Codecov Report

Merging #5160 (34f3f65) into develop (75a01ee) will increase coverage by 0.13%.
The diff coverage is 100.00%.

❗ Current head 34f3f65 differs from pull request most recent head 874f464. Consider uploading reports for the commit 874f464 to get more accurate results

@@             Coverage Diff             @@
##           develop    #5160      +/-   ##
===========================================
+ Coverage    84.35%   84.49%   +0.13%     
===========================================
  Files          148      150       +2     
  Lines        18278    18449     +171     
===========================================
+ Hits         15418    15588     +170     
- Misses        2860     2861       +1

Impacted Files	Coverage Δ
lambdas/indexer/app.py	`95.11% <100.00%> (-0.04%)`	⬇️
src/azul/openapi/params.py	`100.00% <100.00%> (ø)`
src/azul/openapi/spec.py	`100.00% <100.00%> (ø)`

... and 4 files with indirect coverage changes

nadove-ucsc · 2023-04-26T01:15:36Z

New spec is visible here

achave11-ucsc

Nicely done!

hannes-ucsc · 2023-04-27T16:35:25Z

src/azul/openapi/spec.py

+
+
+@attr.s(auto_attribs=True, frozen=True, kw_only=True)
+class AuxSpec:


Suggested change

class AuxSpec:

class CommonEndpointSpecs:

hannes-ucsc · 2023-04-27T16:35:53Z

src/azul/openapi/spec.py

+        return {
+            'method_spec': {
+
+                'summary': 'Describe current version of the Azul service',


hannes-ucsc · 2023-04-27T16:36:55Z

src/azul/openapi/spec.py

+        return {
+            'method_spec': {
+
+                'summary': 'Return OpenAPI specifications for this service',


Suggested change

'summary': 'Return OpenAPI specifications for this service',

'summary': 'Return OpenAPI specifications for this REST API',

Please check if there are more of these cases, where documentation isn't general enough or needs to be parameterized.

hannes-ucsc · 2023-04-27T16:38:30Z

src/azul/openapi/spec.py

+            'tags': ['Auxiliary']
+        }
+
+    def full_health(self) -> JSON:


Many of these can be properties.

hannes-ucsc · 2023-04-27T16:39:25Z

lambdas/indexer/app.py

 def basic_health():
    return app.health_controller.basic_health()


-@app.route('/health/cached', methods=['GET'], cors=True)
+@app.route('/health/cached', methods=['GET'], cors=True, **aux_spec.cached_health())


L here and in several other places.

hannes-ucsc · 2023-04-27T16:39:42Z

lambdas/indexer/app.py

@@ -103,14 +110,17 @@ def static_resource(file):
    return app.swagger_resource(file)


-@app.route('/openapi', methods=['GET'], cors=True)
+aux_spec = AuxSpec(app_name='indexer')


Suggested change

aux_spec = AuxSpec(app_name='indexer')

common_specs = AuxSpec(app_name='indexer')

hannes-ucsc · 2023-04-27T16:50:12Z

lambdas/indexer/app.py

+        'description': 'Contents of the notification',
+        'required': True,
+        **json_content(schema.object(
+            match=schema.object(


I believe this conflicts with PR #5059 for #5000.

That's correct. I'll avoid requesting concurrent reviews for these PRs to avoid conflicts.

hannes-ucsc · 2023-04-27T16:52:21Z

lambdas/indexer/app.py

@@ -151,7 +161,43 @@ def update_health_cache(_event: chalice.app.CloudWatchEvent):
    app.health_controller.update_cache()


-@app.route('/{catalog}/{action}', methods=['POST'])
+@app.route('/{catalog}/{action}', methods=['POST'], method_spec={


This should mention the HMAC header and briefly explain how to compute it.

achave11-ucsc · 2023-05-23T23:19:26Z

PR failed sandbox and anvilbox pipelines.

nadove-ucsc · 2023-05-24T01:06:38Z

Due to my changes, generate_openapi_document script patches either the service or the indexer endpoint depending on which module (service or indexer) it is targeting. However, AzulChaliceApp always uses the service endpoint when constructing base_url, so when the script patches the indexer endpoint, it allows the unpatched service endpoint to slip through. I believe the solution is to modify the base_url property so that ServiceApp and IndexerApp can each use their correct respective endpoint.

github-actions bot added the orange [process] Done by the Azul team label Apr 26, 2023

nadove-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch 2 times, most recently from f37ad27 to 8e2f848 Compare April 26, 2023 00:56

nadove-ucsc requested a review from achave11-ucsc April 26, 2023 01:23

nadove-ucsc assigned achave11-ucsc Apr 26, 2023

achave11-ucsc removed their assignment Apr 26, 2023

nadove-ucsc assigned achave11-ucsc Apr 26, 2023

achave11-ucsc previously approved these changes Apr 27, 2023

View reviewed changes

achave11-ucsc removed their assignment Apr 27, 2023

nadove-ucsc dismissed achave11-ucsc’s stale review via b8ff22d April 27, 2023 01:27

nadove-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch from 74117a4 to b8ff22d Compare April 27, 2023 01:27

nadove-ucsc requested a review from achave11-ucsc April 27, 2023 02:00

nadove-ucsc assigned achave11-ucsc Apr 27, 2023

achave11-ucsc previously approved these changes Apr 27, 2023

View reviewed changes

achave11-ucsc marked this pull request as ready for review April 27, 2023 02:32

achave11-ucsc requested a review from hannes-ucsc as a code owner April 27, 2023 02:32

achave11-ucsc assigned hannes-ucsc and unassigned achave11-ucsc Apr 27, 2023

hannes-ucsc requested changes Apr 27, 2023

View reviewed changes

hannes-ucsc removed their assignment Apr 27, 2023

hannes-ucsc added the 1 review [process] Lead requested changes once label Apr 27, 2023

nadove-ucsc dismissed achave11-ucsc’s stale review via 0a01a03 May 2, 2023 03:46

nadove-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch from b8ff22d to 0a01a03 Compare May 2, 2023 03:46

nadove-ucsc changed the base branch from develop to issues/noah-aviel-dove/5000-index-supp-files-anvil May 2, 2023 03:46

nadove-ucsc added the chained [process] PR needs to based of develop before merging label May 2, 2023

nadove-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch from 0a01a03 to d94c936 Compare May 2, 2023 04:05

hannes-ucsc removed their assignment May 23, 2023

achave11-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch from 3db488f to ffabb56 Compare May 23, 2023 23:08

achave11-ucsc added the sandbox [process] Resolution is being verified in sandbox deployment label May 23, 2023

achave11-ucsc removed their assignment May 23, 2023

achave11-ucsc removed the sandbox [process] Resolution is being verified in sandbox deployment label May 23, 2023

nadove-ucsc dismissed hannes-ucsc’s stale review via 2bf7de8 May 25, 2023 02:15

nadove-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch 3 times, most recently from d55cf96 to 59b9011 Compare May 25, 2023 06:44

nadove-ucsc added 3 commits May 25, 2023 19:31

Fix: Builds pass after failing to generate openapi document (#5228)

8b6e2f7

Move openapi make target to lambda dir

0088143

Reimplement URL patch in OpenAPI script

1e68954

nadove-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch from 59b9011 to 52f1a38 Compare May 26, 2023 02:38

nadove-ucsc requested a review from hannes-ucsc May 26, 2023 03:05

nadove-ucsc assigned hannes-ucsc May 26, 2023

hannes-ucsc added 4+ reviews [process] Lead requested changes four times or more and removed 3 reviews [process] Lead requested changes thrice labels May 26, 2023

hannes-ucsc assigned achave11-ucsc and unassigned hannes-ucsc May 26, 2023

nadove-ucsc added 3 commits May 26, 2023 10:24

Generate openapi document for indexer

fca3e53

Move auxiliary spec from service app to openapi module

f52ae5d

Fix: Swagger UI for indexer is a stub (#5051)

874f464

achave11-ucsc force-pushed the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch from 52f1a38 to 874f464 Compare May 26, 2023 17:25

achave11-ucsc added the sandbox [process] Resolution is being verified in sandbox deployment label May 26, 2023

hannes-ucsc approved these changes May 26, 2023

View reviewed changes

achave11-ucsc merged commit 61f6e0d into develop May 26, 2023

achave11-ucsc deleted the issues/nadove-ucsc/5051-swagger-ui-indexer-stub branch May 27, 2023 00:38

achave11-ucsc removed their assignment May 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Swagger UI for indexer is a stub (#5051, #5228) #5160

Fix: Swagger UI for indexer is a stub (#5051, #5228) #5160

nadove-ucsc commented Apr 26, 2023 •

edited by achave11-ucsc

Loading

coveralls commented Apr 26, 2023 •

edited

Loading

codecov bot commented Apr 26, 2023 •

edited

Loading

nadove-ucsc commented Apr 26, 2023

achave11-ucsc left a comment

hannes-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

nadove-ucsc Apr 27, 2023

hannes-ucsc Apr 27, 2023

achave11-ucsc commented May 23, 2023

nadove-ucsc commented May 24, 2023



		@attr.s(auto_attribs=True, frozen=True, kw_only=True)
		class AuxSpec:

	'summary': 'Return OpenAPI specifications for this service',
	'summary': 'Return OpenAPI specifications for this REST API',

	aux_spec = AuxSpec(app_name='indexer')
	common_specs = AuxSpec(app_name='indexer')

Fix: Swagger UI for indexer is a stub (#5051, #5228) #5160

Fix: Swagger UI for indexer is a stub (#5051, #5228) #5160

Conversation

nadove-ucsc commented Apr 26, 2023 • edited by achave11-ucsc Loading

Checklist

Author

Author (reindex, API changes)

Author (chains)

Author (upgrading)

Author (operator tasks)

Author (hotfixes)

Author (before every review)

Peer reviewer (after requesting changes)

Peer reviewer (after approval)

Primary reviewer (after requesting changes)

Primary reviewer (after approval)

Operator (before pushing merge the commit)

Operator (after pushing the merge commit)

Operator (reindex)

Operator

Shorthand for review comments

coveralls commented Apr 26, 2023 • edited Loading

codecov bot commented Apr 26, 2023 • edited Loading

Codecov Report

nadove-ucsc commented Apr 26, 2023

achave11-ucsc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achave11-ucsc commented May 23, 2023

nadove-ucsc commented May 24, 2023

nadove-ucsc commented Apr 26, 2023 •

edited by achave11-ucsc

Loading

coveralls commented Apr 26, 2023 •

edited

Loading

codecov bot commented Apr 26, 2023 •

edited

Loading