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

fix: type linting issue #7789

Merged
merged 5 commits into from
Apr 19, 2022
Merged

Conversation

glasnt
Copy link
Contributor

@glasnt glasnt commented Apr 12, 2022

Description

A fix for #7787, more to come.

Borrows solution from #7788 regarding Any types.

Checklist

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Apr 12, 2022
@glasnt glasnt changed the title fix: type linting issue (borrows from 7788) fix: type linting issue Apr 12, 2022
@glasnt glasnt marked this pull request as ready for review April 12, 2022 23:19
@glasnt glasnt requested review from a team as code owners April 12, 2022 23:19
Copy link

@ace-n ace-n left a comment

Choose a reason for hiding this comment

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

LGTM from a Cloud Run (not Python) lens.

@glasnt glasnt merged commit 8168040 into GoogleCloudPlatform:main Apr 19, 2022
@glasnt glasnt deleted the topic/deployment-types branch April 19, 2022 01:16
@glasnt glasnt mentioned this pull request Apr 19, 2022
8 tasks
gcf-merge-on-green bot pushed a commit that referenced this pull request Apr 20, 2022
## Description

#7342 introduced a change where nox would check for `test_*.py` and `*_test.py` files. Django and IDP-SQL had their tests in `test/`, so this glob didn't find these files, and these tests haven't been run for a bit. 



Outstanding issues with these tests: 

*dynamic PR status update follows*

---


 * Django: Postgres 9.6 EOL

```
Step #6 - "Migrate database": django.db.utils.ProgrammingError: column c.relispartition does not exist
Step #6 - "Migrate database": LINE 3:             CASE WHEN c.relispartition THEN 'p' WHEN c.relki...
Step #6 - "Migrate database": 
```

Debugging seems to indicate this happens on old versions of postgres. [Django 4.0 no longer supports Postgres 9.6](https://docs.djangoproject.com/en/4.0/releases/4.0/#dropped-support-for-postgresql-9-6). `test-instance-pg` is Postgres 9.6, which is EOL as of November 2021. 

Suggest creating a newer versioned instance of a postgres testing database (happy to help here where I can).

Update: test-instance-pg has been updated to Postgres 14

---

* IDP: Linting issues

```
./middleware.py:31:29: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                            ^
./middleware.py:31:42: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                         ^
./middleware.py:31:61: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                                            ^
```

This is being addressed as of v2.9.0 of flake8-annotations. sco1/flake8-annotations#136 

It will require adding `--allow-star-arg-any` to the flake8 call. 

Update: while this flag could be used, I opted to use the `TypeVar` method in #7789

---

## Checklist
- [ ] I have followed [Sample Guidelines from AUTHORING_GUIDE.MD](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md)
- [ ] README is updated to include [all relevant information](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#readme-file)
- [ ] **Tests** pass:   `nox -s py-3.6` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] **Lint** pass:   `nox -s lint` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones)
- [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones)
- [ ] Please **merge** this PR for me once it is approved.
- [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/CODEOWNERS) with the codeowners for this sample
leahecole pushed a commit that referenced this pull request May 25, 2022
* Fix type linting issuse (borrows from 7788)

* Correct import

Co-authored-by: Ace Nassri <[email protected]>
leahecole pushed a commit that referenced this pull request May 25, 2022
## Description

#7342 introduced a change where nox would check for `test_*.py` and `*_test.py` files. Django and IDP-SQL had their tests in `test/`, so this glob didn't find these files, and these tests haven't been run for a bit. 



Outstanding issues with these tests: 

*dynamic PR status update follows*

---


 * Django: Postgres 9.6 EOL

```
Step #6 - "Migrate database": django.db.utils.ProgrammingError: column c.relispartition does not exist
Step #6 - "Migrate database": LINE 3:             CASE WHEN c.relispartition THEN 'p' WHEN c.relki...
Step #6 - "Migrate database": 
```

Debugging seems to indicate this happens on old versions of postgres. [Django 4.0 no longer supports Postgres 9.6](https://docs.djangoproject.com/en/4.0/releases/4.0/#dropped-support-for-postgresql-9-6). `test-instance-pg` is Postgres 9.6, which is EOL as of November 2021. 

Suggest creating a newer versioned instance of a postgres testing database (happy to help here where I can).

Update: test-instance-pg has been updated to Postgres 14

---

* IDP: Linting issues

```
./middleware.py:31:29: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                            ^
./middleware.py:31:42: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                         ^
./middleware.py:31:61: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                                            ^
```

This is being addressed as of v2.9.0 of flake8-annotations. sco1/flake8-annotations#136 

It will require adding `--allow-star-arg-any` to the flake8 call. 

Update: while this flag could be used, I opted to use the `TypeVar` method in #7789

---

## Checklist
- [ ] I have followed [Sample Guidelines from AUTHORING_GUIDE.MD](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md)
- [ ] README is updated to include [all relevant information](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#readme-file)
- [ ] **Tests** pass:   `nox -s py-3.6` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] **Lint** pass:   `nox -s lint` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones)
- [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones)
- [ ] Please **merge** this PR for me once it is approved.
- [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/CODEOWNERS) with the codeowners for this sample
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants