-
Notifications
You must be signed in to change notification settings - Fork 41
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: Ensure compatible table schema by adding missing columns and expanding column sizes #327
base: main
Are you sure you want to change the base?
Feat: Ensure compatible table schema by adding missing columns and expanding column sizes #327
Conversation
WalkthroughWalkthroughThe recent updates to the Changes
Possibly related issues
Wdyt? Would you like to add any specific details or changes to the summary? Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedRuff
Additional comments not posted (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (17)
airbyte/_future_cdk/sql_processor.py (17)
447-447
: Line too longThe line exceeds the maximum line length of 100 characters.
- alterations.append(self._generate_alter_column_statement(table_name, column_name, new_column_spec)) + alterations.append( + self._generate_alter_column_statement(table_name, column_name, new_column_spec) + )Tools
Ruff
447-447: Line too long (119 > 100)
(E501)
453-458
: Docstring formattingMulti-line docstring summary should start at the first line and remove whitespace after opening quotes.
- """ - Retrieve the schema for the specified stream. - + """Retrieve the schema for the specified stream. +Tools
Ruff
453-458: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
455-455: Blank line contains whitespace
Remove whitespace from blank line
(W293)
457-457: Line too long (111 > 100)
(E501)
455-455
: Remove whitespace from blank lineRemove the unnecessary whitespace from the blank line.
- +Tools
Ruff
455-455: Blank line contains whitespace
Remove whitespace from blank line
(W293)
457-457
: Line too longThe line exceeds the maximum line length of 100 characters.
- :return: Dictionary of the stream's schema with column names as keys and their specifications as values + :return: Dictionary of the stream's schema with column names as keys and specifications as valuesTools
Ruff
457-457: Line too long (111 > 100)
(E501)
463-468
: Docstring formattingMulti-line docstring summary should start at the first line and remove whitespace after opening quotes.
- """ - Retrieve the schema of the specified table. - + """Retrieve the schema of the specified table. +Tools
Ruff
463-468: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
465-465: Blank line contains whitespace
Remove whitespace from blank line
(W293)
467-467: Line too long (107 > 100)
(E501)
465-465
: Remove whitespace from blank lineRemove the unnecessary whitespace from the blank line.
- +Tools
Ruff
465-465: Blank line contains whitespace
Remove whitespace from blank line
(W293)
467-467
: Line too longThe line exceeds the maximum line length of 100 characters.
- :return: Dictionary of existing schema with column names as keys and their specifications as values + :return: Dictionary of existing schema with column names as keys and specifications as valuesTools
Ruff
467-467: Line too long (107 > 100)
(E501)
474-474
: Use double quotesSingle quotes found but double quotes are preferred.
- schema[row['Field']] = row + schema[row["Field"]] = rowTools
Ruff
474-474: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
478-484
: Docstring formattingMulti-line docstring summary should start at the first line and remove whitespace after opening quotes.
- """ - Check if the column size needs to be expanded. - + """Check if the column size needs to be expanded. +Tools
Ruff
478-484: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
480-480: Blank line contains whitespace
Remove whitespace from blank line
(W293)
480-480
: Remove whitespace from blank lineRemove the unnecessary whitespace from the blank line.
- +Tools
Ruff
480-480: Blank line contains whitespace
Remove whitespace from blank line
(W293)
485-490
: Use double quotesSingle quotes found but double quotes are preferred.
- existing_type = existing_spec['Type'] - new_type = new_spec['Type'] - - if '(' in existing_type and '(' in new_type: - existing_size = int(existing_type.split('(')[1].rstrip(')')) - new_size = int(new_type.split('(')[1].rstrip(')')) + existing_type = existing_spec["Type"] + new_type = new_spec["Type"] + + if "(" in existing_type and "(" in new_type: + existing_size = int(existing_type.split("(")[1].rstrip(")")) + new_size = int(new_type.split("(")[1].rstrip(")"))Tools
Ruff
485-485: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
486-486: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
487-487: Blank line contains whitespace
Remove whitespace from blank line
(W293)
488-488: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
488-488: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
489-489: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
489-489: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
490-490: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
490-490: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
495-502
: Docstring formattingMulti-line docstring summary should start at the first line and remove whitespace after opening quotes.
- """ - Generate an ALTER TABLE statement for expanding column size. - + """Generate an ALTER TABLE statement for expanding column size. +Tools
Ruff
495-502: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
497-497: Blank line contains whitespace
Remove whitespace from blank line
(W293)
497-497
: Remove whitespace from blank lineRemove the unnecessary whitespace from the blank line.
- +Tools
Ruff
497-497: Blank line contains whitespace
Remove whitespace from blank line
(W293)
503-503
: Use double quotesSingle quotes found but double quotes are preferred.
- new_type = column_spec['Type'] + new_type = column_spec["Type"]Tools
Ruff
503-503: Single quotes found but double quotes preferred
Replace single quotes with double quotes
(Q000)
494-494
: Line too longThe line exceeds the maximum line length of 100 characters.
- :param column_spec: New column specification :return: ALTER TABLE statement as a string + :param column_spec: New column specification + :return: ALTER TABLE statement as a stringTools
Ruff
494-494: Line too long (108 > 100)
(E501)
507-511
: Docstring formattingMulti-line docstring summary should start at the first line and remove whitespace after opening quotes.
- """ - Execute a list of ALTER TABLE statements. - + """Execute a list of ALTER TABLE statements. +Tools
Ruff
507-511: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
509-509: Blank line contains whitespace
Remove whitespace from blank line
(W293)
509-509
: Remove whitespace from blank lineRemove the unnecessary whitespace from the blank line.
- +Tools
Ruff
509-509: Blank line contains whitespace
Remove whitespace from blank line
(W293)
/fix-pr
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
airbyte/_future_cdk/sql_processor.py (1)
469-469
: Fix long line issue.The line exceeds the maximum allowed length of 100 characters.
- :return: Dictionary of existing schema with column names as keys and their specifications as values + :return: Dictionary of existing schema with column names as keys and their specifications + as valuesTools
Ruff
469-469: Line too long (107 > 100)
(E501)
airbyte/_future_cdk/sql_processor.py
Outdated
def _get_stream_schema(self, stream_name: str) -> dict: | ||
"""Retrieve the schema for the specified stream. | ||
|
||
:param stream_name: Name of the stream | ||
:return: Dictionary of the stream's schema with column names as keys and their specifications as values | ||
""" | ||
# Implement this method to fetch the schema from the stream | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implement _get_stream_schema
method.
The _get_stream_schema
method is not implemented. This method should fetch the schema from the stream.
Would you like me to help with implementing this method or open a GitHub issue to track this task?
Tools
Ruff
460-460: Line too long (111 > 100)
(E501)
@GovindHede - This is exciting! Thanks for contributing to PyAirbyte. 🙏 It looks like great progress here and I'd be excited to merge this when ready. Lmk by tagging me if you need any assistance. I applied the In case it is helpful, you can use these CI jobs to check any issues:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Summary
This pull request ensures that the table schema is compatible with the stream's schema by adding any missing columns and expanding column sizes if necessary.
Changes Made
_ensure_compatible_table_schema
method to:_get_stream_schema
: Retrieve the schema for the specified stream._get_table_schema
: Retrieve the schema of the specified table._is_size_expansion_needed
: Check if the column size needs to be expanded._generate_alter_column_statement
: Generate an ALTER TABLE statement for expanding column size._execute_alterations
: Execute a list of ALTER TABLE statements.Issue Reference
Summary by CodeRabbit
New Features
Bug Fixes
These changes improve the system's adaptability to schema changes, enhancing overall reliability.