Closed mshadbolt closed 2 months ago
We shouldn't merge this until https://github.com/CanDIG/katsu/pull/238 is approved and merged, will need to update the katsu submodule too
I also have a couple of comments that are out of scope for this PR:
test_ingest_not_admin_katsu
and test_ingest_admin_katsu
, are becoming quite large.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 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
- Some test functions, such as
test_ingest_not_admin_katsu
andtest_ingest_admin_katsu
, are becoming quite large.
Do you have a suggestion for how to split them up?
- 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?
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']]):
Incorporates changes in katsu and ingest to allow program curators to ingest into katsu
Also includes upgrading pip to version 24 because the github action build started failing and this seemed to fix it