refactor: remove v1 legacy code #1185
Conversation
olevski
force-pushed
the
remove-v1-code
branch
from
eb8fa56 to
255177f
Compare
|
You can access the deployment of this PR at https://renku-ci-ds-1185.dev.renku.ch |
| if self.default_resource_pool_file is not None: | ||
| with open(self.default_resource_pool_file) as f: | ||
| self.default_resource_pool = crc_models.UnsavedResourcePool(**safe_load(f)) | ||
| if ( | ||
| self.config.server_options.defaults_path is not None | ||
| and self.config.server_options.ui_choices_path is not None | ||
| ): | ||
| with open(self.config.server_options.ui_choices_path) as f: | ||
| options = ServerOptions.model_validate(safe_load(f)) | ||
| with open(self.config.server_options.defaults_path) as f: | ||
| defaults = ServerOptionsDefaults.model_validate(safe_load(f)) | ||
| self.default_resource_pool = generate_default_resource_pool(options, defaults) |
There was a problem hiding this comment.
This needs to be documented: are deployments not getting a default resource pool now?
There was a problem hiding this comment.
This is all for v1. For v1 we had these config files with options kind of like a resource pool. So we generated the default resource pool from them if the file was present. But we also have a default resource pool hardcoded that remains and will be added to the db for v2. I can remove one more field the default_resource_pool_file from the dependency manager.
components/renku_data_services/notebooks/api/amalthea_patches/git_proxy.py
Outdated
Show resolved
Hide resolved
| options: ServerOptions = field(default_factory=lambda: ServerOptions(0.5, 1, 0, 1, "/lab", False, True)) | ||
|
|
||
| async def validate_class_storage(self, user: APIUser, class_id: int, storage: int | None = None) -> ServerOptions: | ||
| async def validate_class_storage(self, user: APIUser, class_id: int, storage: int | None = None) -> None: |
There was a problem hiding this comment.
If this is doing nothing, why are the method and its calls not removed from the code?
There was a problem hiding this comment.
It does nothing in the DummyCRCValidator which is just used for testing. In the actual implementation in the same file (which is still needed) it checks that the resource class exists and is accessible and that the storage requested for the session is compatible with the resource class.
There was a problem hiding this comment.
It used to also return this ServerOptions object that was only needed in V1. But it is still being called from the session starting code for v2 and the output in the v2 code is not used at all.
components/renku_data_services/notebooks/api/classes/k8s_client.py
Outdated
Show resolved
Hide resolved
olevski
force-pushed
the
remove-v1-code
branch
from
255177f to
15e2f95
Compare
There was a problem hiding this comment.
Note: this is because I updated sanic. I will check if we can restrict query parsing or if we can express that on the API spec.
There was a problem hiding this comment.
I added a fix for it. May not be super pretty but it works. It just needed to forbid extra params. And yeah I figured it was because of the sanic update.
There was a problem hiding this comment.
But that's not a good fix, as the permissive query validation will happen on all the endpoints with query parameters. I will see if the @validate_query decorator can be adjusted to please schemathesis.
Pull Request Test Coverage Report for Build 21256253913Details
💛 - Coveralls |
|
Tearing down the temporary RenkuLab deployment for this PR. |
/deploy