Skip to content

Conversation

@Fokko
Copy link
Contributor

@Fokko Fokko commented Dec 3, 2023

No description provided.

@Fokko Fokko mentioned this pull request Dec 3, 2023
Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the quick fix!

existing_schema_id: Optional[int] = None
for schema in self._table.metadata.schemas:
if new_schema == schema:
existing_schema_id = schema.schema_id
Copy link
Contributor

@HonahX HonahX Dec 3, 2023

Choose a reason for hiding this comment

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

Shall we add a break here? Or may be:

existing_schema_id = next(
        (schema.schema_id for schema in self.schemas if schema == new_schema), None)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, your suggestion is beautiful, thanks!

@Fokko Fokko merged commit fc0a72f into apache:main Dec 4, 2023
@Fokko Fokko deleted the fd-fix-downgrade-schema branch December 4, 2023 03:47
@Fokko
Copy link
Contributor Author

Fokko commented Dec 4, 2023

Thanks @danielcweeks and @HonahX for the review 🙌

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