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

Consolidate update/delete custom definitions to regular update/delete definitions #19669

Closed
xiaohansong opened this issue Nov 21, 2022 · 3 comments
Assignees

Comments

@xiaohansong
Copy link
Contributor

xiaohansong commented Nov 21, 2022

FE should use the same API when user wants to update or delete their source or destination definition.

This ticket requires to consolidate 4 custom APIs into the original API (FE will use the one without custom in it, such as updateSourceDefinition)

Also as part of the effort we should remove the legacy creation path to avoid misuse:

POST /v1/source_definitions/create
POST /v1/destination_definitions/create

@pmossman
Copy link
Contributor

Refining notes:

  • Before removing any routes, we should check to see if there are other places that call them. For instance, does Octavia CLI call POST /v1/source_definitions/create or POST /v1/destination_definitions/create? @alafanechere perhaps you know the answer to this?

davinchia pushed a commit that referenced this issue Nov 23, 2022
Closes #19637

PR that introduced this bug was a precision revert of another issue: #19627

This PR removes the createSourceDefinition() and createDestinationDefinition() methods in favor of the new createCustomSourceDefinition() and createCustomDestinationDefinition() methods. createSourceDefinition() and createDestinationDefinition() are to be deprecated.

This PR does not address updating custom connectors. There is a backend issue for fixing this behavior here: #19669
@marcelopio
Copy link
Contributor

marcelopio commented Nov 23, 2022

I was doing a PR to implement custom definitions, so as far as I know, octavia do not call it yet. My implementation is using the endpoints with the "custom" prefix

#15058

class SourceDefinition(SourceAndDestinationDefinition):
api = source_definition_api.SourceDefinitionApi
create_function_name = "create_custom_source_definition"
resource_id_field = "source_definition_id"
get_function_name = "get_source_definition"
update_function_name = "update_custom_source_definition"
resource_type = "source_definition"
@property
def create_payload(self):
definition = SourceDefinitionCreate(self.resource_name, self.docker_repository, self.docker_image_tag, self.documentation_url)
return CustomSourceDefinitionCreate(self.workspace_id, definition)
@property
def get_payload(self) -> Optional[SourceDefinitionIdRequestBody]:
"""Defines the payload to retrieve the remote source definition if a state exists.
Returns:
SourceDefinitionIdRequestBody: The SourceDefinitionIdRequestBody payload.
"""
if self.state is not None:
return SourceDefinitionIdRequestBody(self.state.resource_id)
@property
def update_payload(self):
definition = SourceDefinitionUpdate(source_definition_id=self.resource_id, docker_image_tag=self.docker_image_tag)
return CustomSourceDefinitionUpdate(self.workspace_id, definition)

SofiiaZaitseva pushed a commit that referenced this issue Nov 24, 2022
Closes #19637

PR that introduced this bug was a precision revert of another issue: #19627

This PR removes the createSourceDefinition() and createDestinationDefinition() methods in favor of the new createCustomSourceDefinition() and createCustomDestinationDefinition() methods. createSourceDefinition() and createDestinationDefinition() are to be deprecated.

This PR does not address updating custom connectors. There is a backend issue for fixing this behavior here: #19669
@pmossman
Copy link
Contributor

Closing this as a duplicate in favor of #19670

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants