Skip to content

Additional endpoints - storagebox & key#57

Open
glorpen wants to merge 8 commits intoaszlig:masterfrom
glorpen:additional_endpoints
Open

Additional endpoints - storagebox & key#57
glorpen wants to merge 8 commits intoaszlig:masterfrom
glorpen:additional_endpoints

Conversation

@glorpen
Copy link

@glorpen glorpen commented Dec 15, 2022

Storageboxes implementation is forked from #38, with small code cleanup and added linked_storagebox field in Servers.

Added support for (ssh) keys endpoint.

@glorpen glorpen force-pushed the additional_endpoints branch from 6c127db to 6320beb Compare December 15, 2022 19:03
@glorpen glorpen force-pushed the additional_endpoints branch from 6320beb to c3a80b3 Compare December 15, 2022 19:04
Copy link
Owner

@aszlig aszlig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot. I currently don't have a storage box so I can't test this, but apart from the Py2/3 stuff mentioned, LGTM.

Comment on lines +5 to +9
name: str
fingerprint: str
size: int
data: str
created_at: datetime.datetime
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding type hints while at the same time using deprecated stuff such as class Foo(object): and super(Key, self) is somewhat inconsistent. However, since even I nowadays use type hints and more modern features in other projects, I'm wondering whether it's time to even drop support for Python 2 for this project. That however has to happen for a new major version, though.

So either we stick here to being 2.x compatible and merge this pull request or we use Python 3.x features (consistently) here but wait for the next major release.

def __init__(self, conn):
self.conn = conn

def get(self, id_):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've used box_id in storagebox.py, which not only is more consistent but also doesn't come with the underscore-suffix which I consider to be a workaround.

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.

3 participants