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

What to do with tuf/exceptions before 1.0.0? #1729

Closed
MVrachev opened this issue Dec 14, 2021 · 6 comments
Closed

What to do with tuf/exceptions before 1.0.0? #1729

MVrachev opened this issue Dec 14, 2021 · 6 comments
Assignees
Labels
1.0.0 blockers To be addressed before 1.0.0 release discussion Discussions related to the design, implementation and operation of the project
Milestone

Comments

@MVrachev
Copy link
Collaborator

Description of issue or feature request:

Currently, we are using tuf/exceptions.py to store client and repository exceptions both for the new and old implementation.
After we drop the old code we will have to keep the exceptions from tuf/exceptions.py or move them to somewhere else.
Some of the options I and @jku come up with are the following:

  1. create a new exceptions file in tuf/api/exceptions.py storing all exceptions needed from the client, metadata API, and future repository code (see #17256). Then when the old code is dropped the tuf/exceptons.py will be removed as well.
  2. keep tuf/exceptions.py and just remove the old exceptions.py after we remove the old code. This will require linting the file and changing the docstrinsg (see Start linting tuf/exceptions.py with all linters #1721)
  3. create new exceptions files: one in tuf/api/ with repository side and metadata API exceptions and one in tuf/ngclient/ where we will store the exceptions for the ngclient. Remove tuf/exceptions.py together with the rest of the old code.

Option 1 or 2 seem easier to change with less code, but option 3 will provide better encapsulation

@MVrachev
Copy link
Collaborator Author

MVrachev commented Dec 14, 2021

@lukpueh, @jku do you have an opinion?

@lukpueh
Copy link
Member

lukpueh commented Dec 15, 2021

I don't have a strong opinion here, but I tend towards option 3. Also, I wonder if we still want to keep the api directory once we've removed legacy code. IIRC api was a bit of a crutch so we don't mix up new and legacy code.

@lukpueh
Copy link
Member

lukpueh commented Dec 15, 2021

I think it is more important to figure out which (custom or built-in) exceptions we raise where (see #1131, #1312, #967) than where we define custom exceptions.

@jku jku changed the title What to do with tuf/exceptions after 1.0.0? What to do with tuf/exceptions before 1.0.0? Dec 15, 2021
@jku
Copy link
Member

jku commented Dec 15, 2021

(changing title, I assume this was intended)

@MVrachev
Copy link
Collaborator Author

MVrachev commented Dec 17, 2021

In #1725 I have tried implementing option 1.
I don't like option 2 as we will have to wait to drop the old code before we can actually work on it freely.
Option 3 is not bad, but the only problem is that in tuf/metadata/api.py there is code used both in ngclient
and most likely will be used in the future repository code - for example Metadata.from_file().

Also, I wonder if we still want to keep the api directory once we've removed legacy code. IIRC api was a bit of a crutch so we don't mix up new and legacy code.

My personal feelings are against moving the code anywhere for the following reasons:

  • the path tuf/api or tuf/ngclient is not long, so at least I wasn't bothered by it
  • the bigger reason is that we will lose the git history. I went through that when researching how to rename the old test files. See tests: Start linting tests #1582 (comment)

@lukpueh and @jku can you see #1725 and #1734 which seem related to the current discussion.

@MVrachev MVrachev self-assigned this Jan 11, 2022
@MVrachev MVrachev added 1.0.0 blockers To be addressed before 1.0.0 release discussion Discussions related to the design, implementation and operation of the project labels Jan 11, 2022
@MVrachev MVrachev added this to the Sprint 15 milestone Jan 12, 2022
@MVrachev
Copy link
Collaborator Author

We have decided in #1725 that we would place the new exception file in tuf/api/exceptions.py, so this issue is solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0.0 blockers To be addressed before 1.0.0 release discussion Discussions related to the design, implementation and operation of the project
Projects
None yet
Development

No branches or pull requests

3 participants