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

ci(starlette): unpin sqlalchemy in framework tests [backport #4801 to 1 7] #4833

Merged
merged 1 commit into from
Dec 22, 2022

Conversation

mabdinur
Copy link
Contributor

Description

backport: #4801

The starlette framework tests currently pin sqlalchemy to 1.4.41 due to
an upstream issue which has now closed:
encode/databases#512. It looks like we can
unpin this dependency.

Note: this also indirectly fixes the failing starlette framework tests
due to this now closed upstream issue:
encode/databases#513, where sqlalchemy==1.4.42
added a new private field in the SQLAlchemy cursor class. While this job
uses the latest databases package that has accounted for this field,
this was causing attribute errors since we were using a previous version
of sqlalchemy that did not yet have that private field added.

Checklist

Motivation

Design

Testing strategy

Relevant issue(s)

Testing strategy

Reviewer Checklist

  • Title is accurate.
  • Description motivates each change.
  • No unnecessary changes were introduced in this PR.
  • Avoid breaking
    API
    changes unless absolutely necessary.
  • Tests provided or description of manual testing performed is
    included in the code or PR.
  • Release note has been added and follows the library release note
    guidelines
    ,
    or else changelog/no-changelog label added.
  • All relevant GitHub issues are correctly linked.
  • Backports are identified and tagged with Mergifyio.

## Description
The starlette framework tests currently pin sqlalchemy to 1.4.41 due to
an upstream issue which has now closed:
encode/databases#512. It looks like we can
unpin this dependency.

Note: this also indirectly fixes the failing starlette framework tests
due to this now closed upstream issue:
encode/databases#513, where sqlalchemy==1.4.42
added a new private field in the SQLAlchemy cursor class. While this job
uses the latest databases package that has accounted for this field,
this was causing attribute errors since we were using a previous version
of sqlalchemy that did not yet have that private field added.


<!-- If this is a breaking change, explain why it is necessary. Breaking
changes must append `!` after the type/scope. See
https://ddtrace.readthedocs.io/en/stable/contributing.html for more
details. -->

## Checklist
- [ ] Followed the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines)
when writing a release note.
- [ ] Add additional sections for `feat` and `fix` pull requests.
- [ ] [Library
documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs)
and/or [Datadog's documentation
site](https://github.com/DataDog/documentation/) is updated. Link to doc
PR in description.

<!-- Copy and paste the relevant snippet based on the type of pull
request -->

<!-- START feat -->

## Motivation
<!-- Expand on why the change is required, include relevant context for
reviewers -->

## Design 
<!-- Include benefits from the change as well as possible drawbacks and
trade-offs -->

## Testing strategy
<!-- Describe the automated tests and/or the steps for manual testing.

<!-- END feat -->

<!-- START fix -->

## Relevant issue(s)
<!-- Link the pull request to any issues related to the fix. Use
keywords for links to automate closing the issues once the pull request
is merged. -->

## Testing strategy
<!-- Describe any added regression tests and/or the manual testing
performed. -->

<!-- END fix -->

## Reviewer Checklist
- [x] Title is accurate.
- [x] Description motivates each change.
- [x] No unnecessary changes were introduced in this PR.
- [x] Avoid breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Tests provided or description of manual testing performed is
included in the code or PR.
- [x] Release note has been added and follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines),
or else `changelog/no-changelog` label added.
- [x] All relevant GitHub issues are correctly linked.
- [x] Backports are identified and tagged with Mergifyio.
@mabdinur mabdinur requested a review from a team as a code owner December 22, 2022 15:51
@mabdinur mabdinur changed the title fix(starlette framework tests): unpin sqlalchemy [backport #4801 to 1 7] ci(starlette): unpin sqlalchemy in framework tests [backport #4801 to 1 7] Dec 22, 2022
@mabdinur mabdinur added the changelog/no-changelog A changelog entry is not required for this PR. label Dec 22, 2022
@mabdinur mabdinur changed the base branch from 1.x to 1.7 December 22, 2022 16:11
@mabdinur mabdinur requested a review from a team as a code owner December 22, 2022 16:11
@Yun-Kim Yun-Kim merged commit 0ef51f1 into 1.7 Dec 22, 2022
@Yun-Kim Yun-Kim deleted the backport-4801-to-1-7 branch December 22, 2022 16:58
@github-actions github-actions bot added this to the v1.7.0 milestone Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants