Fix: Documents are not sliced during manifest generation (#5967) by nadove-ucsc · Pull Request #5968 · DataBiosphere/azul

nadove-ucsc · 2024-02-17T01:56:50Z

Connected issues: #5967

Checklist

Author

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

Author (partiality)

Added p tag to titles of partial commits
Added partial label to PR _{or this PR completely resolves all connected issues}
All connected issues are resolved partially _{or this PR does not have the partial label}

¹ 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}
PR and connected issue are labeled API _{or this PR does not modify a REST API}
Added a (A) tag to commit title for backwards (in)compatible changes _{or this PR does not modify a REST API}
Updated REST API version number in app.py _{or this PR does not modify a REST 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 deployments)

Ran make image_manifests.json and committed any resulting changes _{or this PR does not modify azul_docker_images or any other variables referenced in the definition of that variable}
Documented upgrading of deployments in UPGRADING.rst _{or this PR does not require upgrading deployments}
Added u tag to commit title _{or this PR does not require upgrading deployments}
Added upgrade label to PR _{or this PR does not require upgrading deployments}

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 system administrator
PR is assigned to system administrator

System administrator (after requesting changes)

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

System administrator (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
PR is assigned to current operator

Operator (before pushing merge the commit)

Operator (chain shortening)

Changed the target branch of the blocked PR to develop _{or this PR is not labeled base}
Removed the chained label from the blocked PR _{or this PR is not labeled base}
Removed the blocking relationship from the blocked PR _{or this PR is not labeled base}
Removed the base label from this PR _{or this PR is not labeled base}

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)

Operator

PR is assigned to no one

Shorthand for review comments

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

codecov · 2024-02-17T02:16:53Z

Codecov Report

Attention: Patch coverage is 95.83333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 85.17%. Comparing base (a44b350) to head (3fe7c4f).

Files	Patch %	Lines
test/integration_test.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #5968   +/-   ##
========================================
  Coverage    85.17%   85.17%           
========================================
  Files          154      154           
  Lines        19893    19898    +5     
========================================
+ Hits         16943    16948    +5     
  Misses        2950     2950

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

coveralls · 2024-02-17T02:17:10Z

coverage: 85.192% (+0.003%) from 85.189%
when pulling 3fe7c4f on issues/nadove-ucsc/5967-documents-not-sliced
into a44b350 on develop.

nadove-ucsc · 2024-02-28T10:01:39Z

First commit resolves python-attrs/attrs#1081 which would become a problem on the 2nd commit

achave11-ucsc

Some convention nits, otherwise, LGTM!

src/azul/indexer/document.py

dsotirho-ucsc · 2024-03-01T18:55:10Z

Failed IT on anvilbox and hammerbox twice each.

======================================================================
ERROR: test_indexing (integration_test.IndexingIntegrationTest.test_indexing) [manifest] (catalog='anvil-it', format=<ManifestFormat.compact: 'compact'>, fetch=True)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builds/ucsc/azul/test/integration_test.py", line 427, in subTest
…
  File "/build/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 894, in urlopen
    return self.urlopen(
           ^^^^^^^^^^^^^
  [Previous line repeated 2 more times]
  File "/build/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 884, in urlopen
    retries = retries.increment(method, url, response=response, _pool=self)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/.venv/lib/python3.11/site-packages/urllib3/util/retry.py", line 592, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='service.anvilbox.anvil.gi.ucsc.edu', port=443): Max retries exceeded with url: /fetch/manifest/files/k8QgmPY4EJ6F-lYud3vWCH7ksTw2SyNORfWu_Zw23S9DHygAAQ== (Caused by ResponseError('too many 500 error responses'))
======================================================================
ERROR: test_indexing (integration_test.IndexingIntegrationTest.test_indexing) [managed_access_manifest] (catalog='anvil-it')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builds/ucsc/azul/test/integration_test.py", line 427, in subTest
…
  File "/build/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 894, in urlopen
    return self.urlopen(
           ^^^^^^^^^^^^^
  [Previous line repeated 2 more times]
  File "/build/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 884, in urlopen
    retries = retries.increment(method, url, response=response, _pool=self)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/.venv/lib/python3.11/site-packages/urllib3/util/retry.py", line 592, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='service.anvilbox.anvil.gi.ucsc.edu', port=443): Max retries exceeded with url: /manifest/files/k8QgaakyRoQrFgut4wDc2mS4L_-raW4p0pjJc9GQFZAP_dIAAQ== (Caused by ResponseError('too many 500 error responses'))

hannes-ucsc · 2024-03-03T21:35:51Z

Tested against a personal deployment resembling sandbox and another one resembling anvilbox.

...to avoid python-attrs/attrs#1081

This avoids excessive string concatenation when enforcing them during field translation.

github-actions bot added the orange label Feb 17, 2024

nadove-ucsc added the spike:1 [process] Spike estimate of one point label Feb 17, 2024

nadove-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from f2881ef to 24cdb00 Compare February 17, 2024 02:01

nadove-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch 7 times, most recently from 1efdfee to 3cbd080 Compare February 22, 2024 08:06

nadove-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from 3cbd080 to bde95c3 Compare February 28, 2024 09:57

nadove-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from a7358b3 to 33fdb29 Compare February 28, 2024 10:03

nadove-ucsc added the reqs [process] PR includes commit requiring ``make requirements`` label Feb 28, 2024

nadove-ucsc requested a review from achave11-ucsc February 28, 2024 20:18

nadove-ucsc assigned achave11-ucsc Feb 28, 2024

achave11-ucsc requested changes Feb 28, 2024

View reviewed changes

src/azul/indexer/document.py Outdated Show resolved Hide resolved

src/azul/indexer/document.py Outdated Show resolved Hide resolved

achave11-ucsc removed their assignment Feb 28, 2024

nadove-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from 0f80558 to 1b49705 Compare February 29, 2024 01:45

nadove-ucsc requested a review from achave11-ucsc February 29, 2024 02:36

nadove-ucsc assigned achave11-ucsc Feb 29, 2024

achave11-ucsc previously approved these changes Feb 29, 2024

View reviewed changes

achave11-ucsc marked this pull request as ready for review February 29, 2024 02:39

achave11-ucsc requested a review from hannes-ucsc as a code owner February 29, 2024 02:39

achave11-ucsc assigned hannes-ucsc and unassigned achave11-ucsc Feb 29, 2024

hannes-ucsc dismissed achave11-ucsc’s stale review via 4166683 March 1, 2024 02:38

hannes-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from 1bd8865 to 4166683 Compare March 1, 2024 02:38

hannes-ucsc removed their assignment Mar 1, 2024

dsotirho-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from 6238300 to 4e5ac02 Compare March 1, 2024 16:47

dsotirho-ucsc added the sandbox [process] Resolution is being verified in sandbox deployment label Mar 1, 2024

dsotirho-ucsc assigned hannes-ucsc and unassigned dsotirho-ucsc Mar 1, 2024

hannes-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from 2973957 to b1dd698 Compare March 2, 2024 20:10

hannes-ucsc previously approved these changes Mar 3, 2024

View reviewed changes

hannes-ucsc assigned dsotirho-ucsc and unassigned hannes-ucsc Mar 3, 2024

hannes-ucsc and others added 12 commits March 4, 2024 17:50

Work around false type warning

758b838

Fix another type warning

94275a8

Fix wording of docstring

97bdc5a

[R] Upgrade to attrs==23.1.0

6e3e3a2

...to avoid python-attrs/attrs#1081

Fix UnboundLocalError after subtest fails

67caed6

Assert document slicing

4c1dad8

Use tuples to represent paths of included fields

dc97ab0

This avoids excessive string concatenation when enforcing them during field translation.

Fix: Documents are not sliced during manifest generation (#5967)

7cd7bcf

Fix: KeyError when generating compact manifest for AnVIL

9f42c95

Add useful/missing columns to AnVIL manifest

42ec50a

Fix type warning and reliance on undocumented string conversion

c711621

[R] Ran make requirements_update

3fe7c4f

dsotirho-ucsc dismissed hannes-ucsc’s stale review via 3fe7c4f March 5, 2024 01:58

dsotirho-ucsc force-pushed the issues/nadove-ucsc/5967-documents-not-sliced branch from b1dd698 to 3fe7c4f Compare March 5, 2024 01:58

hannes-ucsc approved these changes Mar 5, 2024

View reviewed changes

dsotirho-ucsc merged commit 360efa6 into develop Mar 5, 2024

dsotirho-ucsc deleted the issues/nadove-ucsc/5967-documents-not-sliced branch March 5, 2024 05:34

dsotirho-ucsc removed their assignment Mar 5, 2024

hannes-ucsc linked an issue Sep 12, 2025 that may be closed by this pull request

Documents are not sliced during manifest generation #5967

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Documents are not sliced during manifest generation (#5967)#5968

Fix: Documents are not sliced during manifest generation (#5967)#5968
dsotirho-ucsc merged 12 commits intodevelopfrom
issues/nadove-ucsc/5967-documents-not-sliced

nadove-ucsc commented Feb 17, 2024 •

edited by dsotirho-ucsc

Loading

Uh oh!

codecov bot commented Feb 17, 2024 •

edited

Loading

Uh oh!

coveralls commented Feb 17, 2024 •

edited

Loading

Uh oh!

nadove-ucsc commented Feb 28, 2024

Uh oh!

achave11-ucsc left a comment

Uh oh!

Uh oh!

Uh oh!

dsotirho-ucsc commented Mar 1, 2024

Uh oh!

hannes-ucsc commented Mar 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

nadove-ucsc commented Feb 17, 2024 • edited by dsotirho-ucsc Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Author

Author (partiality)

Author (reindex, API changes)

Author (chains)

Author (upgrading deployments)

Author (operator tasks)

Author (hotfixes)

Author (before every review)

Peer reviewer (after requesting changes)

Peer reviewer (after approval)

System administrator (after requesting changes)

System administrator (after approval)

Operator (before pushing merge the commit)

Operator (chain shortening)

Operator (after pushing the merge commit)

Operator (reindex)

Operator

Shorthand for review comments

Uh oh!

codecov bot commented Feb 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coveralls commented Feb 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nadove-ucsc commented Feb 28, 2024

Uh oh!

achave11-ucsc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dsotirho-ucsc commented Mar 1, 2024

Uh oh!

hannes-ucsc commented Mar 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nadove-ucsc commented Feb 17, 2024 •

edited by dsotirho-ucsc

Loading

codecov bot commented Feb 17, 2024 •

edited

Loading

coveralls commented Feb 17, 2024 •

edited

Loading