Skip to content

Comments

feature/rbac#130

Merged
bwalsh merged 3 commits intodevelopmentfrom
feature/rbac
Jul 21, 2025
Merged

feature/rbac#130
bwalsh merged 3 commits intodevelopmentfrom
feature/rbac

Conversation

@bwalsh
Copy link
Collaborator

@bwalsh bwalsh commented Jul 9, 2025

@bwalsh bwalsh requested a review from Copilot July 9, 2025 01:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new unit test to verify that Gen3Index.get includes the authentication provider and bumps the package version.

  • Introduces test_authorization_header_present in tests/unit/test_indexclient.py
  • Updates package version to 0.0.7rc15

Reviewed Changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated 1 comment.

File Description
tests/unit/test_indexclient.py Added test for auth provider being passed to requests.get
setup.py Bumped version from 0.0.7rc14 to 0.0.7rc15
Comments suppressed due to low confidence (1)

tests/unit/test_indexclient.py:17

  • [nitpick] The test name implies checking an HTTP header but it actually verifies the auth object insertion; consider renaming to test_auth_provider_passed_to_requests for clarity.
def test_authorization_header_present(index_client: Gen3Index):

@bwalsh
Copy link
Collaborator Author

bwalsh commented Jul 9, 2025

Our first attempt was to fork indexclient - see https://github.com/ohsu-comp-bio/indexclient/tree/feature/rbac
This worked - as far as g3t was concerned

However - because this relied on having a dependency to a git repo, pypi was preventing us from creating a new release see pypi/warehouse#9404

Therefore, we patched indexclient in g3t, thereby removing the dependency on our fork

The same unit test works.

Re-visit this change when/if related PRs are merged:

Feature-change:

Dependencies:

Copy link
Contributor

@quinnwai quinnwai left a comment

Choose a reason for hiding this comment

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

Works as expected when building from source (pip install -e .) but running into issue using version directly from pypi (pip install gen3-tracker==g3t-0.0.7rc15). Specifically, on g3t push...

Publishing \/Users/wongq/data/cbds-demo/test/g3t-0.0.7rc15/lib/python3.13/site-packages/pydantic/main.py:519: UserWarning: Pydantic serializer warnings:
  PydanticSerializationUnexpectedValue(Expected `<class 'pathlib._local.Path'>` but got `<class 'NoneType'>` with value `'None'` - serialized value may not be as expected.)
  PydanticSerializationUnexpectedValue(Expected `<class 'pathlib._local.Path'>` but got `<class 'NoneType'>` with value `'None'` - serialized value may not be as expected.)
  return self.__pydantic_serializer__.to_json(
[2025-07-17 17:05:00,953][WARNING] backoff: call gen3.jobs.async_create_job(<gen3.jobs.Gen3Jobs object at 0x111812120>, fhir_import_export, {'push': {'commits': [{'commit_id': '307c333a-b442-5c05-b630-3d1177bd24a8', 'object_id': '307c333a-b442-5c05-b630-3d1177bd24a8', 'message': 'From g3t-git', 'resource_counts': None, 'exceptions': None, 'logs': None, 'path': None, 'manifest_sqlite_path': None, 'meta_path': '_cbds-qwqw-20250717-170459_meta.zip'}], 'published_timestamp': None, 'published_job': None}, 'project_id': 'cbds-qwqw', 'method': 'put'}) delay 0.4 seconds after 1 tries
Publishing \[2025-07-17 17:05:01,401][WARNING] backoff: call gen3.jobs.async_create_job(<gen3.jobs.Gen3Jobs object at 0x111812120>, fhir_import_export, {'push': {'commits': [{'commit_id': '307c333a-b442-5c05-b630-3d1177bd24a8', 'object_id': '307c333a-b442-5c05-b630-3d1177bd24a8', 'message': 'From g3t-git', 'resource_counts': None, 'exceptions': None, 'logs': None, 'path': None, 'manifest_sqlite_path': None, 'meta_path': '_cbds-qwqw-20250717-170459_meta.zip'}], 'published_timestamp': None, 'published_job': None}, 'project_id': 'cbds-qwqw', 'method': 'put'}) delay 1.2 seconds after 2 tries
Publishing |[2025-07-17 17:05:02,647][  ERROR] backoff: gave up call gen3.jobs.async_create_job(<gen3.jobs.Gen3Jobs object at 0x111812120>, fhir_import_export, {'push': {'commits': [{'commit_id': '307c333a-b442-5c05-b630-3d1177bd24a8', 'object_id': '307c333a-b442-5c05-b630-3d1177bd24a8', 'message': 'From g3t-git', 'resource_counts': None, 'exceptions': None, 'logs': None, 'path': None, 'manifest_sqlite_path': None, 'meta_path': '_cbds-qwqw-20250717-170459_meta.zip'}], 'published_timestamp': None, 'published_job': None}, 'project_id': 'cbds-qwqw', 'method': 'put'}) after 3 tries; exception: (<class 'aiohttp.client_exceptions.ClientConnectorCertificateError'>, ClientConnectorCertificateError(ConnectionKey(host='calypr-dev.ohsu.edu', port=443, is_ssl=True, ssl=True, proxy=None, proxy_auth=None, proxy_headers_hash=None), SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate in certificate chain (_ssl.c:1028)')), <traceback object at 0x1130e5480>)
Unable to publish project. See logs/publish.log for more info
Cannot connect to host calypr-dev.ohsu.edu:443 ssl:True [SSLCertVerificationError: (1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate in certificate chain (_ssl.c:1028)')]

g3t pull an existing project works (gdc-lung) for both

@bwalsh
Copy link
Collaborator Author

bwalsh commented Jul 18, 2025

Hmm. That's an ssl certificate error.
Should have nothing to do with this change?

Copy link
Contributor

@quinnwai quinnwai left a comment

Choose a reason for hiding this comment

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

Cool repushed, works as expected. Uncertain where that came from. Tested that on calypr-dev with indexd changes applied...

  • pushing a single file works
  • pulling gdc-esca
  • deleting a project

🚢 it

@bwalsh bwalsh merged commit 2e85f60 into development Jul 21, 2025
0 of 5 checks passed
quinnwai added a commit that referenced this pull request Aug 12, 2025
* adds no-bucket

* adds test push/clone no-bucket

* adds .gitignore, README.md (#63)

* improve pull output

* improve logs

* test scp use case

* foreign bucket

* data-import-test/file-1MB.1.txt

* improve test

* improve documentation

* handle un-assigned policy

* adds optional request_id to sign

* adds .gitignore, README.md (#63)

* handle un-assigned policy

* adds optional request_id to sign

* read project_id from config

* bump

* typo

* bump

* handle iso dates

* improve fhirdatetime

* continue if exception signing

* Fix/pull from meta (#74)

* pull based on client side meta

* typo

* fix/rm-user (#70)

* bump

* add current project

* bump

* log sign errors

* bump

* Fix/merge no bucket (#76)

* deprecate

* improve timestamps

* flake

* --no-verify

stdout

* refactor, use git

* make dtale optional install

* capture warnings

* make dtale optional install

* delegate unhandled commands to git

* startup time via lazy load

* improve tests

* fix when no patient

* empty, rm project

* improve constant names

* determine hash_type

* --no-server (testing)

* improve tests

* improve ls command

* improves non-gen3 upload

* cmd list

* add-steward

* hides add-steward

* comment

* Adds test_bundle

* adds test_change_file

* fix no project_id

* comment

* fix msg

* fix step = publish

* improve steward ux

* make collaborator add more flexible

* PR feedback

* refactors package name

* make dtale optional

* improve dist instructions

* typo

* adds quickstart

* PR feedback

* improves publish

* migrate legacy projects

* tweak logging

* adds support for legacy meta.zip

* improve add doc

* improve add doc

* improve add doc

* fix typo

* ensure yaml in distro

* account for existing meta

* observation df

* improve observation.component

* bump

* improve clone when no md5

* ensure dvc.meta

* allow existing .git; optional add

* fix bundle

* update df tests

* bump

* bump

* add dup check

* improve messages

* check extensions

* improve observation

* bump

* simplify specimen parent

* improve extension

* uniq id component observations

* bump

* codings now array

* bump

* minor dataframe changes

* fix: optional specimen fields

* bump

* bump

* changes for etl pod

* make cp function work with server code

* swap server support for snapshotter function

* bump version

* zip file

* path type checking

* fix context manager

* type fixing

* dataframe improvements

* dataframe bugfixes

* adds bucket cli

* adds comments

* fix: double slash

* dataframer fixes from data changes

* initial pivot checkin

* Update dataframer to work with pivot and data

* fix

* updates

* Adds unit, subject reference normalization

* add special etl for demo

* remove dbg print

* ensure object_id persisted

* bump

* bugfix

* fix setup version

* fix gethostname

* fix no_bucket

* fix no_bucket

* fix pull remote

* bump

* fix scp

* file:////  4 slashes

* bump

* fastq mime

* bump

* add focus level granularity

* cleanup

* cleanup

* make util work with server code

* Add debug option

* pivot improvements

* adds condition, minor improvements

* fixes related to displaying subject, codes

* bug fixes

* temp remove unit handler

* temp remove unit handler

* create flattened_research_subjects

* update sqlite query

* clean up generator

* enable client-side dataframe creation of research subject

* cleanup

* bump to 0.0.5rc2

* version alrady taken bump to 0.0.5rc3

* Adds stateful LocalFHIRDatabase

* experimental: adds example of smmart dataframe

* fixure data to test usecases (#88)

* fixure data to test usecases

* format to ndjson

* new fixtures, components

* comments

* update identifiers to meet server side validation requirements

* update data

* metadata

* adds SimplifiedDocumentReference

* clarify _populate_simplified_extension

* port over normalize methods and simplifiedFHIR classes; refactor tests

* improve misc policies

improve existing requests

* create new flattened_document_reference(); abstract into entities

* update non-smmart test to call flatenned_document_reference

* reformat fixtures

* use cursor, clean up docref flatten

* observation dataframer WIP; misc cleanup

* fix some unit tests to run using `pytest tests/`

* populate subject observation dataframe with subject data; bugfix obs; clean up docref dataframer

* clean up SimplifiedFHIR classes; rework observation values to handle components and top-level values

* remove connection close from flatteners

* sqlite connection changes

* update docref dataframer to query for docref-focus observations only once; remove docref.focus code

* enable multiple observation with same docref focus, update tests

* version bump

* actual version bump

* remove test dataframer code, as it's already integrated into actual code

* create flattened_specimens method; create flattened_observation helper

* add specimen.subject to dataframe

* remove print statement

* remove redundant tests

* linting

* Walsh: clean up dataframe test file; create specimen test

* simplify fixtures; remove mention of smmart within file; add research subject test

* clarify get_observations_by_focus; refactor get_nested_values and make use of it

* improve get_subject

* MP comments: multi-focus error handling and guppy normalize util fn

* linter

* tiny patch on assert

* fix warning on test, rename test dataframe file

* remove observation dataframing in code and tests, cleanup observation focus code

* update dataframer cli

* initial checkin

* Adds bundle delete, test to client

* Removes sheepdog/peregrine from g3t

* features/stateful emitter (#87)

* Adds stateful LocalFHIRDatabase

* Create SimplifiedFHIR builder object for FHIR object manipulation

* Create SimplifiedObservation and SimplifiedDocumentReference with function overrides

* Create fixtures to mock out current use cases

* clarify _populate_simplified_extension

* documentReference w basedOn - reference -> specimen fixture (#90)

* documentReference w basedOn - reference -> specimen fixture

* Feature/docref observation focus (#93)

* use SimplifiedFHIR classes in DocumentReference flattener

* create Specimen flattener populated with subject and observation metadata

* remove Observation flattener in favor of Specimen flattener

* fix misc unit tests to run using `pytest tests/`

* Consolidated tests

* Create Specimen and ResearchSubject dataframer tests

* Improve error handling

* Update dataframer CLI

---------

Co-authored-by: teslajoy <nasim@plenary.org>
Co-authored-by: Quinn Wai Wong <54592956+quinnwai@users.noreply.github.com>

* Adds flake8 unit test, fixes flake8 errors

* update research subject to use SimplifiedResource

* temp commit

* initial research subject update, requires Condition.ndjson for pytest fixture

* add SimplifiedCondition to override codings() method

* update research subject to use SimplifiedResource

* rebase conflict resolve

* bugfix test

* flake8

* test fixture for condition and associated references (#96)

Includes new Condition, Observation and Encounter metadata

* simplify pulling observation values; clarify get_resource_by_reference

* fix pytests (still need to test new condition fixture); linting

* Revert change to create_id

* specimen added after patient exists

* improve resource.id checks

review feedback

* better-ping

* check grip response, include in exception

* include bucket, access in ping

* update pypi

* improve error reporting

* repeat add changed file

* optionally validate ids

* G3T_NUM_PARALLEL (simultaneous uploads)

* push

* Add initial Github Actions workflows

* [WIP] Feature/docref additions (#98)

* Add system level category key/value for muli element codings

* Add support for multiple identifiers

* update research subject to use SimplifiedResource

* temp commit

* initial research subject update, requires Condition.ndjson for pytest fixture

* add SimplifiedCondition to override codings() method

* update research subject to use SimplifiedResource

* rebase conflict resolve

* test fixture for condition and associated references (#96)

Includes new Condition, Observation and Encounter metadata

* simplify pulling observation values; clarify get_resource_by_reference

* simplify dataframer command line args

* add SimplifiedCondition to override codings() method

* fix pytests (still need to test new condition fixture); linting

* Add system level category key/value for muli element codings

* add SimplifiedCondition to override codings() method

* fix tests

* fix tests

* make data loadable

* Add observation code

* enable multiple focus references; allow specified dataframer output_path

* improve error handling

* improve tests and write logs to file

* add expected invalid docref fixture

* Preserve functionality through backend bulk loader swap

---------

Co-authored-by: quinnwai <quinnwai.wong@gmail.com>
Co-authored-by: Nasim Sanati (Rieker) <nasim@plenary.org>

* Release/0.0.7rc2 (#107)

* update pytest and version number

* linting

* ensure dataframer unit tests pass

* fix test

* fix test to work with new output

* fix spacing

---------

Co-authored-by: matthewpeterkort <matthewpeterkort@gmail.com>

* Feature/dataframer medication administration (#112)

* update pytest and version number

* linting

* ensure dataframer unit tests pass

* fix test

* fix test to work with new output

* fix spacing

* test improved validation

* ensure monkey patches no longer necessary

* datetimes w/out time now return XXXXT00:00:00

* fhir.resources==8

* Initial checkin med admin tab

* fix identifier to avoid hyphen in column name

* Release/0.0.7rc2 (#107)

* update pytest and version number

* linting

* ensure dataframer unit tests pass

* fix test

* fix test to work with new output

* fix spacing

---------

Co-authored-by: matthewpeterkort <matthewpeterkort@gmail.com>

* update pytest and version number

* linting

* hardcode a solution fun

* revert fhir.resources version

* bump version

---------

Co-authored-by: quinnwai <quinnwai.wong@gmail.com>
Co-authored-by: Brian Walsh <brian@bwalsh.com>
Co-authored-by: Quinn Wai Wong <54592956+quinnwai@users.noreply.github.com>

* TKEDTE-351: Feature/group resource (#113)

* add flattener for GroupMember and SimplifiedGroup

* linting

* revert fhir 8.0 changes

* split out simple workflow test from collaborator approving test

* create unique ID as UUID

* create .gitignore to only ignore top level logs directory (#114)

* create .gitignore to only ignore top level logs directory

* remove random print statements

* Feature/parent-specimens (#119)

* get rid of mds_gateway, deprecated?

* dataframe output bugfix

* load in parent specimen as list

* bump version

* fix identifier naming assumptions (#115)

* fix identifier naming assumptions

* clarify identifier

* adds support for granting sower permissions

* fix identifier naming assumptions

* fix incorrect rebase change

* fix test

* fix/rm-uncommitted (#122)

* fix identifier naming assumptions

* clarify identifier

* adds support for granting sower permissions

* fix identifier naming assumptions

* fix incorrect rebase change

* fix test

* improve rm

* adds rm tests

* typo

* validate_document_in_grip/elastic

* check elastic, grip

* optional bundle

* publish fhir if bundle

* publish fhir if bundle

* formatting

* adds test_rm_pushed_links

* clarify warning

* assume -a if no targets; improve warnings

* rm test projects

* remove DocumentReference if no MANIFEST files

* adds --bundle flag

* adds rm

* adds comment

* improve ln -s processing

* handle multiple deleted files

* improve deleted files

* review feedback

* update graph name

* bump release version

* fix name

---------

Co-authored-by: matthewpeterkort <matthewpeterkort@gmail.com>
Co-authored-by: matthewpeterkort <33436238+matthewpeterkort@users.noreply.github.com>

* feature/rbac (#130)

* fixes flake
* adds indexclient rbac branch
* patch indexclient

* feature/multi-hash (#137)

* adds skip-id-check

* ensure all hash types in indexd

* dont check index on dry-run

* fix tests

* bump

---------

Co-authored-by: matthewpeterkort <33436238+matthewpeterkort@users.noreply.github.com>
Co-authored-by: teslajoy <nasim@plenary.org>
Co-authored-by: matthewpeterkort <matthewpeterkort@gmail.com>
Co-authored-by: quinnwai <quinnwai.wong@gmail.com>
Co-authored-by: Quinn Wai Wong <54592956+quinnwai@users.noreply.github.com>
Co-authored-by: Liam Beckman <lbeckman314@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants