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

Drop support for EOL software components #424

Merged
merged 9 commits into from
May 25, 2022
Merged

Drop support for EOL software components #424

merged 9 commits into from
May 25, 2022

Conversation

amotl
Copy link
Member

@amotl amotl commented May 24, 2022

As a precursor to #391, this patch drops support for software components considered to be EOL.

  • Drop support for SQLAlchemy 1.1 and 1.2.
  • Drop support for CrateDB < 2.0.0.
  • Drop support for Python 3.4, 3.5, 3.6.

@amotl amotl force-pushed the amo/modernize branch 3 times, most recently from 3da5030 to 13bb12f Compare May 24, 2022 15:54
@crate crate deleted a comment from lgtm-com bot May 24, 2022
@amotl amotl marked this pull request as ready for review May 24, 2022 16:06
@amotl amotl requested review from mfussenegger and seut May 24, 2022 16:07
mfussenegger
mfussenegger approved these changes May 25, 2022
View reviewed changes
Copy link
Member

@mfussenegger mfussenegger left a comment

Choose a reason for hiding this comment

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

Would be nice to put the change that drops support and the related documentation update into the same commit. Otherwise lgtm

seut
seut reviewed May 25, 2022
View reviewed changes
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

Can't the compiler.CrateCompiler.visit_insert method be removed now as supported CrateDB versions do support insert-from-select without parenthesis?

amotl added 7 commits May 25, 2022 13:53
@amotl
Drop support for SQLAlchemy 1.1 and 1.2
42fe97d
@amotl
Drop support for CrateDB < 1.0.0
c2ce4e5
@amotl
Drop support for CrateDB < 2.0.0
7633b8c
@amotl
Drop support for EOL Python versions, keep Python 3.7 to 3.10
998a141
@amotl
Documentation: Minor edits
695a2cc
@amotl
Documentation: Drop "compatibility notes" section
2d1bc3f 
The compatibility tables would need structural rework in order to
express the "phasing out" of support for specific software versions.

I think it is safe to remove this section completely in order to
reduce maintenance overhead.
@amotl
Remove CrateCompiler.visit_insert method
08ed75c 
It was needed to wrap INSERT statements inside parentheses to conform
with earlier versions of CrateDB. CrateDB >= 1.0.1 already supports
INSERT statements as generated by SQLAlchemy natively, so with the
recent deprecations, it is no longer needed.
@amotl
Copy link
Member Author

amotl commented May 25, 2022

Hi again,

I've amended the commits to include the corresponding updates to the documentation and also added 08ed75c, following up on the comment

For example, when dropping support for CrateDB <= 1.0.0, we could get rid of vendoring the visit_insert compiler method completely. -- #391 (comment)

Thank you for your attention. 🙇‍♂️

With kind regards,
Andreas.

@amotl amotl requested a review from seut May 25, 2022 12:51
seut
seut approved these changes May 25, 2022
View reviewed changes
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

👍

@amotl amotl merged commit 67ec8e3 into master May 25, 2022
@amotl amotl deleted the amo/modernize branch May 25, 2022 13:34
@amotl amotl mentioned this pull request May 25, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seut seut seut approved these changes

@mfussenegger mfussenegger mfussenegger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amotl @mfussenegger @seut