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

feat: Endpoints refreshed with new Request status model and Protobuf models added #9

Merged
merged 15 commits into from
Jul 10, 2024

Conversation

Raalsky
Copy link
Contributor

@Raalsky Raalsky commented Jul 2, 2024

No description provided.

@Raalsky Raalsky changed the title Endpoints refreshed chore: Endpoints refreshed with new Request status model Jul 2, 2024
@Raalsky Raalsky changed the title chore: Endpoints refreshed with new Request status model chore: Endpoints refreshed with new Request status model and Protobuf models added Jul 6, 2024
@Raalsky Raalsky changed the title chore: Endpoints refreshed with new Request status model and Protobuf models added feat: Endpoints refreshed with new Request status model and Protobuf models added Jul 7, 2024

Choose a reason for hiding this comment

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

What's the point of this file? Cannot those imports be in the regular __init__.py?

Copy link
Contributor Author

@Raalsky Raalsky Jul 9, 2024

Choose a reason for hiding this comment

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

I'm just following what protoc and protoletariat tools gives me. The second one fixes the relative imports issue.

@@ -13,7 +13,7 @@
AuthenticatedClient,
Client,
)
from ...models.ingest_response import IngestResponse
from ...proto.neptune_pb.ingest.v1.request_status_pb2 import RequestStatus

Choose a reason for hiding this comment

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

Is it possible to replace such cases with an explicit import like package1.package2.etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this one shouldn't be a problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done I think. There are some that were genered by client generation package that I want to leave it up to the tool itself.

@Raalsky Raalsky merged commit c91d8ed into main Jul 10, 2024
6 checks passed
@Raalsky Raalsky deleted the rj/endpoint-refreshed branch July 10, 2024 09:45
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.

2 participants