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 ci workflow timeout #5408

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Fix ci workflow timeout #5408

merged 1 commit into from
Jan 18, 2021

Conversation

greshilov
Copy link
Contributor

@greshilov greshilov commented Jan 14, 2021

What do these changes do?

Increasing ci workflow timeout.

UPDATE Timeout is not changed, but pip-tools is used to pin dependencies and solve timeout issue.

Are there changes in behavior for the user?

No.

Related issue number

Fixes #5389

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

This change is Reviewable

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jan 14, 2021
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #5408 (6ee2a40) into master (3250c5d) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5408      +/-   ##
==========================================
+ Coverage   97.15%   97.17%   +0.02%     
==========================================
  Files          41       41              
  Lines        8742     8742              
  Branches     1402     1402              
==========================================
+ Hits         8493     8495       +2     
+ Misses        130      129       -1     
+ Partials      119      118       -1     
Flag Coverage Δ
unit 97.06% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiohttp/web.py 99.14% <0.00%> (+1.70%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3250c5d...6ee2a40. Read the comment docs.

@greshilov greshilov force-pushed the pr-5389 branch 2 times, most recently from 6ae77f5 to aef3a17 Compare January 15, 2021 00:47
@webknjaz webknjaz enabled auto-merge (squash) January 15, 2021 01:07
@greshilov greshilov force-pushed the pr-5389 branch 9 times, most recently from e56fa8d to 48d2837 Compare January 15, 2021 02:52
@webknjaz
Copy link
Member

@greshilov Do you want to explore using pip-tools?

@greshilov
Copy link
Contributor Author

@greshilov Do you want to explore using pip-tools?

Looks like i have no choice. I'll try a few more times with constraints.txt, but pip-tools will solve this problem for sure.

@greshilov greshilov force-pushed the pr-5389 branch 4 times, most recently from 036a35c to 9ae468d Compare January 16, 2021 01:59
@greshilov
Copy link
Contributor Author

greshilov commented Jan 16, 2021

@webknjaz pip-tools works like a charm, thank you for suggestioning this package. Please, take a look.

  • dev.txt renamed to dev.in. dev.txt added to .gitignore since it autogenerated.
  • I've updated cryptography version, because pip-compile tool failed to resolve dependencies with old one.
  • Makefile modified accordingly.

@greshilov greshilov changed the title Increase ci workflow timeout Fix ci workflow timeout Jan 16, 2021
@webknjaz
Copy link
Member

  • dev.txt renamed to dev.in. dev.txt added to .gitignore since it autogenerated.

This is a bad idea. Both files must be committed. Moreover, dependabot automatically recognizes the pairs of .in + .txt files and autoupdates the latter.

.gitignore Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
requirements/test.txt Show resolved Hide resolved
@greshilov greshilov force-pushed the pr-5389 branch 3 times, most recently from 7479f42 to 523df30 Compare January 17, 2021 14:10
CHANGES/5389.bugfix Outdated Show resolved Hide resolved
@@ -30,6 +30,8 @@ repos:
exclude: >-
^docs/[^/]*\.svg$
- id: requirements-txt-fixer
exclude: >-
^requirements/dev.txt
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
^requirements/dev.txt
^requirements/dev.txt$

@webknjaz webknjaz merged commit b49fdd5 into aio-libs:master Jan 18, 2021
@pip-compile --allow-unsafe -q requirements/dev.in

.PHONY: install
install: .update-pip
@pip install -Ur requirements/dev.txt
Copy link
Member

Choose a reason for hiding this comment

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

The canonical use of the lockfiles is as follows:

Suggested change
@pip install -Ur requirements/dev.txt
@pip install -r requirements/dev.in -c requirements/dev.txt

@webknjaz
Copy link
Member

Urgh... auto-merge got triggered when I posted comments :(

@webknjaz
Copy link
Member

@greshilov mind backporting this?

@webknjaz
Copy link
Member

Also, it's not clear how well dependabot works with files included via -r. It may be necessary to apply pip-tools there as well.

@greshilov
Copy link
Contributor Author

Urgh... auto-merge got triggered when I posted comments :(

Strange behaviour.

Should I create another PR with hotfixes?

@webknjaz
Copy link
Member

yes, please

greshilov added a commit to greshilov/aiohttp that referenced this pull request Jan 18, 2021
@greshilov greshilov mentioned this pull request Jan 18, 2021
5 tasks
webknjaz pushed a commit that referenced this pull request Jan 18, 2021
@webknjaz webknjaz mentioned this pull request Jan 21, 2021
5 tasks
greshilov added a commit to greshilov/aiohttp that referenced this pull request Jan 21, 2021
alandtse pushed a commit to alandtse/aiohttp that referenced this pull request Feb 14, 2021
* Increase ci workflow timeout

* Fix ci cache key typo

* fixup! Increase ci workflow timeout

* fixup! fixup! Increase ci workflow timeout
alandtse pushed a commit to alandtse/aiohttp that referenced this pull request Feb 14, 2021
commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
* Increase ci workflow timeout

* Fix ci cache key typo

* fixup! Increase ci workflow timeout

* fixup! fixup! Increase ci workflow timeout
commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
* Increase ci workflow timeout

* Fix ci cache key typo

* fixup! Increase ci workflow timeout

* fixup! fixup! Increase ci workflow timeout
commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[maintenance] Adjust test job timeouts in GitHub Actions CI/CD jobs
2 participants