Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DIG-1663: Implement curator ingest in katsu and batch ingest #666

Merged
merged 24 commits into from
Jul 8, 2024

Conversation

mshadbolt
Copy link
Contributor

@mshadbolt mshadbolt commented Jul 2, 2024

Incorporates changes in katsu and ingest to allow program curators to ingest into katsu

  • Fixes integration tests to work with new changes
  • adds program authorization deletion to clean_up methods
  • add to non admin integration test
  • edit add_program_authorization method so that program curators and team members can be added

Also includes upgrading pip to version 24 because the github action build started failing and this seemed to fix it

@mshadbolt mshadbolt requested a review from SonQBChau July 3, 2024 23:22
@mshadbolt mshadbolt marked this pull request as ready for review July 3, 2024 23:22
@mshadbolt
Copy link
Contributor Author

We shouldn't merge this until CanDIG/katsu#238 is approved and merged, will need to update the katsu submodule too

Copy link
Contributor

@SonQBChau SonQBChau left a comment

Choose a reason for hiding this comment

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

Please remove the commits to lib/candigv2-ingest and lib/katsu as they are already merged. This will help us avoid any conflicts.

@SonQBChau
Copy link
Contributor

I also have a couple of comments that are out of scope for this PR:

  1. Some test functions, such as test_ingest_not_admin_katsu and test_ingest_admin_katsu, are becoming quite large.
  2. The naming of roles is confusing. For example, "curators" and "team members" default to the site_admin value. If we need a convenience value, it should be the least privileges, not full privileges. It's easier to catch bugs this way.

@mshadbolt
Copy link
Contributor Author

I also have a couple of comments that are out of scope for this PR:

I don't think they are out of scope, if we want to change them we can incorporate it into this PR

  1. Some test functions, such as test_ingest_not_admin_katsu and test_ingest_admin_katsu, are becoming quite large.

Do you have a suggestion for how to split them up?

  1. The naming of roles is confusing. For example, "curators" and "team members" default to the site_admin value. If we need a convenience value, it should be the least privileges, not full privileges. It's easier to catch bugs this way.

I don't really understand what you mean by this, can you give an example?

@SonQBChau
Copy link
Contributor

Yes, it's out of scope for now, but we can create a ticket to address it during the redesign.

Line 120, it set admin to default user

def add_program_authorization(dataset: str, curators: list = [ENV['CANDIG_SITE_ADMIN_USER']], 
                              team_members: list = [ENV['CANDIG_SITE_ADMIN_USER']]):

@mshadbolt mshadbolt merged commit 3f49bb6 into develop Jul 8, 2024
2 checks passed
@mshadbolt mshadbolt deleted the mshadbolt/katsu-curator branch July 8, 2024 18:26
mshadbolt added a commit that referenced this pull request Jul 8, 2024
* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth
mshadbolt added a commit that referenced this pull request Jul 8, 2024
…nstructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>
mshadbolt added a commit that referenced this pull request Jul 8, 2024
* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth
mshadbolt added a commit that referenced this pull request Jul 8, 2024
…nstructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>
mshadbolt added a commit that referenced this pull request Jul 8, 2024
…inated (#668)

* updating submodules

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

* CanDIG/candigv2-ingest merging: DIG-1658: Update commandline ingest instructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>
mshadbolt added a commit that referenced this pull request Jul 8, 2024
* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth
mshadbolt added a commit that referenced this pull request Jul 8, 2024
…nstructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>
mshadbolt added a commit that referenced this pull request Jul 8, 2024
…inated (#668)

* updating submodules

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

* CanDIG/candigv2-ingest merging: DIG-1658: Update commandline ingest instructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>
mshadbolt added a commit that referenced this pull request Jul 8, 2024
… bugs, and styling changes (#670)

* updating submodules

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

* CanDIG/candigv2-ingest merging: DIG-1658: Update commandline ingest instructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>

* CanDIG/candigv2-query merging: Bugfix: sidebar cohort style pipeDeliminated (#668)

* updating submodules

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

* CanDIG/candigv2-ingest merging: DIG-1658: Update commandline ingest instructions (#667)

* updating submodules

* Update pip

* Update Makefile

* Update requirements.txt

* remove update setuptools

* setuptools first

* Update requirements.txt

* Update example.env

* Update Makefile

* manually update setuptools

* remove setuptools

* Update example.env

* remove setuptools

* updating submodules (#669)

Co-authored-by: github-actions <github-actions@github.com>

* DIG-1663: Implement curator ingest in katsu and batch ingest (#666)

* fix clean up

* update tests

* add 200

* add comment

* update pip

* revert pip

* try pip update

* fix typo

* fix command

* fix command properly

* add #egg

* test names

* fix typos

* update pip

* revert reqs

* fix req

* fix req2

* remove setup upgrade

* remove setuptools upgrade

* delete auth

* update katsu

* update setuptools

* update ingest

* remove default site admin program auth

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.com>

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Marion <mshadbolt@users.noreply.github.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.

None yet

2 participants