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.

Sign up for GitHub

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

Jump to bottom

Refactor the append/merge operator #343

Closed
5 tasks
tatiana opened this issue May 3, 2022 · 1 comment · Fixed by #439
Closed
5 tasks

Refactor the append/merge operator #343

tatiana opened this issue May 3, 2022 · 1 comment · Fixed by #439
Assignees
@kaxil @utkarsharma2
Labels
refactor Refactor code base, without introducing new features
Milestone
0.10.0

Comments

@tatiana
Copy link
Collaborator

tatiana commented May 3, 2022 •
edited
Loading

Describe the context
This task is part of the Astro Python SDK refactor.

Dependencies

Acceptance criteria

  • Implement the append operator using the new Database class and using the create_database(conn_id) function. It should contain both the behavior of the previously existing merge and append.
  • All the quality checks passed
  • All the functions have typehints
  • All the new functions and modules have docstrings
  • The introduced code has 100% of test coverage
@tatiana tatiana added the refactor Refactor code base, without introducing new features label May 3, 2022
@kaxil kaxil added this to the 0.9.0 milestone May 13, 2022
@kaxil kaxil modified the milestones: 0.9.0, 0.10.0 May 24, 2022
@kaxil
Copy link
Collaborator

kaxil commented May 24, 2022

As part of this ticket we should add append/merge for all DBs too

This was referenced May 24, 2022
Closed
Closed
@kaxil kaxil mentioned this issue Jun 2, 2022
Merged
kaxil added a commit that referenced this issue Jun 2, 2022
@kaxil
Refactor MergeOperator (#422)
03f0e99 
This PR refactors the Merge operator to use the new interfaces. This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383 

part of #343
@kaxil kaxil assigned kaxil and utkarsharma2 and unassigned tatiana Jun 7, 2022
kaxil added a commit that referenced this issue Jun 7, 2022
@kaxil @utkarsharma2
Refactor AppendOperator
123df9e 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
@kaxil kaxil mentioned this issue Jun 7, 2022
Merged
kaxil added a commit that referenced this issue Jun 7, 2022
@kaxil @utkarsharma2
Refactor AppendOperator
32c2036 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
kaxil added a commit that referenced this issue Jun 7, 2022
@kaxil @utkarsharma2
Refactor AppendOperator
0ebc21a 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
kaxil added a commit that referenced this issue Jun 7, 2022
@kaxil @utkarsharma2
Refactor AppendOperator
1a9caab 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
kaxil added a commit that referenced this issue Jun 8, 2022
@kaxil @tatiana @utkarsharma2
Refactor AppendOperator
c72de22 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>
Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
kaxil added a commit that referenced this issue Jun 8, 2022
@kaxil @tatiana @utkarsharma2
Refactor AppendOperator
eb60ea3 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>
Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
@kaxil kaxil closed this as completed in #439 Jun 8, 2022
kaxil added a commit that referenced this issue Jun 8, 2022
@kaxil @utkarsharma2
Refactor AppendOperator (#439)
c80b5a2 
This PR refactors the Append operator to use the new interfaces.

Breaking Changes:
- Removed casting functionality
- changed the interface from which uses `source_to_target_columns_map` instead of `columns`:

```
    def append(
        self,
        main_table: Table,
        columns: List[str],
        casted_columns: dict,
        append_table: Table,
    ):
```

to

```
def append(
    source_table: Table,
    target_table: Table,
    source_to_target_columns_map: Optional[Dict[str, str]] = None,
    **kwargs,
):
```

This PR intends to keep the same logic as now and just use new interfaces so we can delete all the old files from old interfaces. The scope to whether or not we should have a single function or not is kept for #383

closes #343
closes #335

Co-Authored-By: Utkarsh Sharma <utkarsharma2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kaxil kaxil

@utkarsharma2 utkarsharma2

Labels
refactor Refactor code base, without introducing new features
Projects
None yet
Milestone
0.10.0
Development

Successfully merging a pull request may close this issue.

Refactor AppendOperator astronomer/astro-sdk
3 participants
@tatiana @kaxil @utkarsharma2