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

[Blocked] Rename user to owner #753

Closed
wants to merge 6 commits into from
Closed

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Jun 23, 2022

Description:

  • Uses the new owner property instead of user since the latter one is deprecated.

Issues:

Todos:

Make sure to delete unnecessary points or to check all before merging:

  • Updated Changelog
  • Updated Documentation
  • Added / Updated Tests
  • Considered adding this to the Examples

@philippotto philippotto self-assigned this Jun 23, 2022
@philippotto
Copy link
Member Author

@jstriebel There are still type errors which I assume to be due to a differing setup for the client generation. These are the errors:

webknossos/client/_download_dataset.py:84: error: "DatasetInfoResponse200DataSourceDataLayersItem" has no attribute "default_view_configuration"  [attr-defined]
webknossos/client/_download_dataset.py:86: error: "DatasetInfoResponse200DataSourceDataLayersItem" has no attribute "default_view_configuration"  [attr-defined]
webknossos/administration/project.py:117: error: Item "Unset" of "Union[Unset, ProjectInfoByIdResponse200Owner, ProjectInfoByNameResponse200Owner]" has no attribute "id"  [union-attr]

I saw that the client generation expects a dataset e2006_knossos to exist. I'm not sure where this is coming from. This dataset is not available on master.webknossos.xyz or webknossos.org. In our notion page, it was uploaded, but that one is in KNOSSOS which can't be right. Locally, I had a e2006_wkw which I renamed to e2006_knossos, but I assume that it is missing a default_view_configuration ? However, I'm also confused by the type error regarding the project info. Maybe you can clarify both points after your vacation :)

@philippotto philippotto requested a review from jstriebel June 28, 2022 15:17
@philippotto philippotto changed the title [WIP] Rename user to owner [Blocked] Rename user to owner Jun 28, 2022
@jstriebel
Copy link
Contributor

@jstriebel There are still type errors which I assume to be due to a differing setup for the client generation. These are the errors:

webknossos/client/_download_dataset.py:84: error: "DatasetInfoResponse200DataSourceDataLayersItem" has no attribute "default_view_configuration"  [attr-defined]
webknossos/client/_download_dataset.py:86: error: "DatasetInfoResponse200DataSourceDataLayersItem" has no attribute "default_view_configuration"  [attr-defined]
webknossos/administration/project.py:117: error: Item "Unset" of "Union[Unset, ProjectInfoByIdResponse200Owner, ProjectInfoByNameResponse200Owner]" has no attribute "id"  [union-attr]

I saw that the client generation expects a dataset e2006_knossos to exist. I'm not sure where this is coming from. This dataset is not available on master.webknossos.xyz or webknossos.org. In our notion page, it was uploaded, but that one is in KNOSSOS which can't be right. Locally, I had a e2006_wkw which I renamed to e2006_knossos, but I assume that it is missing a default_view_configuration ?

This is indeed quite tricky. The __generate_client.py script uses e2006_knossos, because it's in the test-database.

When using the local docker-compose setup from the wk-libs repo (which is spawned automatically, but only if nothing is running on port 9000), then this dataset always exists, as it is checked into this repo.

It's not quite ideal for a setup, but I'm also not sure, how to change it. Maybe it would be best to simply ignore possible local setups by default and use another port for the client generation? I'd be super happy about suggestions and improvements :-)

However, I'm also confused by the type error regarding the project info.

This is a separate issue. Here, the generated client uses a sentinel object (basically a singleton) UNSET of type Unset. You only checked for None in the code, not Unset.


Since I was assigned issue #719, I fixed the same problems on PR #760 and also included the deprecations for the username etc. I'll close this PR therefore, but let's still discuss how to improve the setup.

@jstriebel jstriebel closed this Jul 5, 2022
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.

Rename Annotation user to owner
2 participants