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

Refactor: Rocky KATalogus clients #3697

Closed
Donnype opened this issue Oct 17, 2024 · 1 comment · Fixed by #3717
Closed

Refactor: Rocky KATalogus clients #3697

Donnype opened this issue Oct 17, 2024 · 1 comment · Fixed by #3717
Assignees
Labels
😸 Review/QA feedback Review/QA feedback provided rocky Issues related to Rocky tech-debt technical-design

Comments

@Donnype
Copy link
Contributor

Donnype commented Oct 17, 2024

          Some APIs of those clients are not organization-specific though, and hence forcing an organization as a property of the API client becomes a bit ugly. I believe this client is not the place to force handing over the organization as that makes the clients do to much and make them not reusable in context where we should have full access, such as with the runner. To have a nice layered design, we could introduce a layer where we properly manage access, such as a new client class that can only call organization specific methods of the base client.

Originally posted by @Donnype in #3666 (comment)

@Donnype Donnype self-assigned this Oct 17, 2024
@Donnype Donnype added rocky Issues related to Rocky tech-debt technical-design labels Oct 17, 2024
@Donnype Donnype added this to KAT Oct 17, 2024
@github-project-automation github-project-automation bot moved this to Incoming features / Need assessment in KAT Oct 17, 2024
@Donnype Donnype moved this from Incoming features / Need assessment to Backlog / To do in KAT Oct 17, 2024
@Donnype Donnype mentioned this issue Oct 17, 2024
8 tasks
@underdarknl
Copy link
Contributor

I'd propose to split the client into two classes.
The base class that handles generic (eg, non orga or user bound) requests, which can be initiated without providing the organization code. This layer could also handle most generic functonality (session building, http requests as a private method) of the expected exceptions (eg, timeouts, connection errors) and streamline them into Exception classes that subclass from the Generic KatalogusException or BytesException.

The second class subclasses from this first class and adds the organization or user arguments to the init. It also adds the specific methods that only work when an organzation or client is required. This layer also adds permission checks if needed, and the needed exceptions. (eg, dot bleed a rw http 404 or http 403 back to the user but instead throw something like a KatalogusPermissionDenied or KatalogusObjectNotFound error. (which can be subclassed from the KatalogusException AND the generic objectNotfoud classes available in Django. This helps catching these exceptions everywhere upstream.

@stephanie0x00 stephanie0x00 added the 😸 Review/QA feedback Review/QA feedback provided label Oct 28, 2024
@Donnype Donnype changed the title Refactor: Rocky (organization-specific) clients Refactor: Rocky KATalogus clients Nov 5, 2024
@stephanie0x00 stephanie0x00 moved this from QA review / functional testing to In Progress in KAT Nov 12, 2024
@Donnype Donnype moved this from In Progress to QA review / functional testing in KAT Nov 12, 2024
@stephanie0x00 stephanie0x00 moved this from QA review / functional testing to Ready for merge in KAT Nov 12, 2024
@github-project-automation github-project-automation bot moved this from Ready for merge to Done in KAT Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😸 Review/QA feedback Review/QA feedback provided rocky Issues related to Rocky tech-debt technical-design
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants