Skip to content

PD-97 Add unit test to check CLI/pypfb import#285

Merged
paulineribeyre merged 22 commits intomasterfrom
chore/cli-pfb-test
Mar 3, 2026
Merged

PD-97 Add unit test to check CLI/pypfb import#285
paulineribeyre merged 22 commits intomasterfrom
chore/cli-pfb-test

Conversation

@paulineribeyre
Copy link
Copy Markdown
Contributor

@paulineribeyre paulineribeyre commented Jan 2, 2026 •
edited by k-burt-uch
Loading

Regression test for:

Traceback (most recent call last):
  File "/Users/kyle/.pyenv/versions/3.13.4/bin/gen3", line 5, in <module>
    from gen3.cli.__main__ import main
  File "/Users/kyle/.pyenv/versions/3.13.4/lib/python3.13/site-packages/gen3/cli/__main__.py", line 9, in <module>
    import gen3.cli.pfb as pfb
  File "/Users/kyle/.pyenv/versions/3.13.4/lib/python3.13/site-packages/gen3/cli/pfb.py", line 2, in <module>
    from pfb import cli as pfb_cli
  File "/Users/kyle/.pyenv/versions/3.13.4/lib/python3.13/site-packages/pfb/cli.py", line 63, in <module>
    for ep in entry_points().get("pfb.plugins", []):
              ^^^^^^^^^^^^^^^^^^

Link to JIRA ticket if there is one: https://ctds-planx.atlassian.net/browse/PD-97

New Features

Breaking Changes

Bug Fixes

  • Fixes issue with SDK running in python 3.9

Improvements

Dependency updates

Deployment changes

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 2, 2026

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 2, 2026

filepath passed skipped SUBTOTAL
tests/test_presigned_url.py 7 0 7
tests/test_graph_submit_and_query.py 13 1 14
tests/test_data_upload.py 8 1 9
TOTAL 28 2 30

Please find the detailed integration test report here

Please find the Github Action logs here

@k-burt-uch
Fix entry point loading in >3.9 python for pfb and users
740fc20 
Update test_cli to assert SystemExit 0
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 3, 2026

filepath passed error skipped SUBTOTAL
tests/test_presigned_url.py 7 0 0 7
tests/test_graph_submit_and_query.py 13 0 1 14
tests/test_data_upload.py 8 1 1 10
TOTAL 28 1 2 31

Please find the detailed integration test report here

Please find the Github Action logs here

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 3, 2026

filepath passed skipped SUBTOTAL
tests/test_data_upload.py 8 1 9
TOTAL 8 1 9

Please find the detailed integration test report here

Please find the Github Action logs here

Avantol13
Avantol13 reviewed Feb 4, 2026
View reviewed changes
tests/test_cli.py
import pytest


def test_import_cli(monkeypatch):
Copy link
Copy Markdown
Contributor

@Avantol13 Avantol13 Feb 4, 2026

Choose a reason for hiding this comment

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

Suggested change
def test_import_cli(monkeypatch):
def test_import_cli(monkeypatch):
"""
Test that invoking the help from the CLI does not return an error
"""

Avantol13
Avantol13 reviewed Feb 4, 2026
View reviewed changes
gen3/utils.py
# Load plug-ins from entry_points (syntax changes for python 3.12+)
major = sys.version_info[0]
minor = sys.version_info[1]
if major == 3 and minor >= 12:
Copy link
Copy Markdown
Contributor

@Avantol13 Avantol13 Feb 4, 2026

Choose a reason for hiding this comment

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

I would much rather try/catch than explicitly name major and minor versions in the code, but I don't feel super strongly about it. It's just that if in the long long future Python releases a 4 major, this is going to break again.

Avantol13
Avantol13 previously approved these changes Feb 4, 2026
View reviewed changes
paulineribeyre
paulineribeyre commented Feb 4, 2026
View reviewed changes
gen3/utils.py Outdated
Comment on lines +354 to +358
for ep in entry_points(group="pfb.plugins"):
ep.load()
else:
for ep in entry_points().get("pfb.plugins", []):
ep.load()
Copy link
Copy Markdown
Contributor Author

@paulineribeyre paulineribeyre Feb 4, 2026

Choose a reason for hiding this comment

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

i didn't look into how this works exactly, but it looks off to me that it's hardcoded to pfb.plugins here and called in both gen3/cli/pfb.py and gen3/cli/users.py

Copy link
Copy Markdown
Contributor

@k-burt-uch k-burt-uch Mar 3, 2026

Choose a reason for hiding this comment

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

Fixed. It should be gen3.plugins not pfb.plugins. Thanks for catching the copy paste error

@k-burt-uch k-burt-uch closed this Mar 3, 2026
@k-burt-uch k-burt-uch reopened this Mar 3, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Failed to Prepare CI environment

Please find the Github Action logs here

@k-burt-uch k-burt-uch closed this Mar 3, 2026
@k-burt-uch k-burt-uch reopened this Mar 3, 2026
@paulineribeyre
Copy link
Copy Markdown
Contributor Author

paulineribeyre commented Mar 3, 2026

See https://cdis.slack.com/archives/C0294A7KVE0/p1772573412435139?thread_ts=1772558196.769849&cid=C0294A7KVE0

@paulineribeyre paulineribeyre merged commit 869248d into master Mar 3, 2026
16 of 18 checks passed
@paulineribeyre paulineribeyre deleted the chore/cli-pfb-test branch March 3, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@k-burt-uch k-burt-uch k-burt-uch left review comments

@Avantol13 Avantol13 Avantol13 left review comments

@nrandorf nrandorf Awaiting requested review from nrandorf

Assignees

No one assigned

Labels

TestDataUpload

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@paulineribeyre @Avantol13 @k-burt-uch @m0nhawk