Skip to content

bpo-42332: Add weakref slot to types.GenericAlias#23250

Merged
miss-islington merged 3 commits into
python:masterfrom
Fidget-Spinner:ga-weakref
Nov 16, 2020
Merged

bpo-42332: Add weakref slot to types.GenericAlias#23250
miss-islington merged 3 commits into
python:masterfrom
Fidget-Spinner:ga-weakref

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented Nov 12, 2020

Copy link
Copy Markdown
Member

https://bugs.python.org/issue42332

Automerge-Triggered-By: GH:gvanrossum

@Tinche

Tinche commented Nov 12, 2020

Copy link
Copy Markdown
Contributor

I created the issue, and I have no experience setting those fields. It'd be great to get it in the next 3.9 release though.

@Tinche

Tinche commented Nov 12, 2020

Copy link
Copy Markdown
Contributor

Also, thank you for implementing this!

@Fidget-Spinner

Copy link
Copy Markdown
Member Author

You're welcome! Personally, I'd like to see this on the next 3.9 release too, I'll just see what the core devs decide.

Btw, I have a feeling that arguments on whether to backport or not should go onto the bpo thread, so that it's easier for others to follow.

@jstasiak jstasiak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two small comments but looks good to me otherwise.

Comment thread Lib/test/test_genericalias.py
Comment thread Lib/test/test_genericalias.py Outdated

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LG, one style nit.

Comment thread Lib/test/test_genericalias.py Outdated
Comment thread Objects/genericaliasobject.c

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good!

@miss-islington

Copy link
Copy Markdown
Contributor

@Fidget-Spinner: Status check is done, and it's a failure ❌ .

@Fidget-Spinner

Fidget-Spinner commented Nov 16, 2020

Copy link
Copy Markdown
Member Author

Azure Pipelines tests are failing due to WinArm64 tests failing (I didn't even know we had tests for ARM!) the full error log is here.
Here's a snippet:

c1 : fatal error C1083: Cannot open source file: 'D:\a\1\b\externals\sqlite-3.33.0.0\sqlite3.c': No such file or directory [D:\a\1\s\PCbuild\sqlite3.vcxproj]
    3 Warning(s)
    1 Error(s)

Time Elapsed 00:03:34.57
##[error]Cmd.exe exited with code '1'.

Seems like a problem with solution file for ARM. All the other tests are passing though. From cpython's repository commit history, this PR might be the first ever tested with WinArm64 on azure pipelines, I'm guessing it's a new test with things not fully ironed out yet?

EDIT:
Nevermind I saw the start of the log

Traceback (most recent call last):
  File "D:\a\1\s\PCbuild\\get_external.py", line 60, in <module>
    main()
  File "D:\a\1\s\PCbuild\\get_external.py", line 48, in main
    zip_path = fetch_zip(
  File "D:\a\1\s\PCbuild\\get_external.py", line 17, in fetch_zip
    filename, headers = urlretrieve(
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 247, in urlretrieve
    with contextlib.closing(urlopen(url, data)) as fp:
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 222, in urlopen
    return opener.open(url, data, timeout)
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 531, in open
    response = meth(req, response)
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 640, in http_response
    response = self.parent.error(
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 569, in error
    return self._call_chain(*args)
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 502, in _call_chain
    result = func(*args)
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\urllib\request.py", line 649, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 504: Gateway Time-out

get_externals.bat wasn't successful due to connection issues.

@Fidget-Spinner

Copy link
Copy Markdown
Member Author

Sorry @gvanrossum, is it possible for you to trigger a re-run of the Azure Pipelines Github Action? Or do I have to push an empty commit?

@gvanrossum

Copy link
Copy Markdown
Member

I don’t know how to trigger just that, but I will close and then reopen.

@gvanrossum gvanrossum closed this Nov 16, 2020
@gvanrossum

Copy link
Copy Markdown
Member

You may also try rebasing, in case it’s fixed in main.

@gvanrossum gvanrossum reopened this Nov 16, 2020
@miss-islington

Copy link
Copy Markdown
Contributor

@Fidget-Spinner: Status check is done, and it's a failure ❌ .

1 similar comment
@miss-islington

Copy link
Copy Markdown
Contributor

@Fidget-Spinner: Status check is done, and it's a failure ❌ .

@gvanrossum

Copy link
Copy Markdown
Member

Hopefully miss islington will eventually get it.

@miss-islington

Copy link
Copy Markdown
Contributor

@Fidget-Spinner: Status check is done, and it's a success ✅ .

@miss-islington
miss-islington merged commit 384b7a4 into python:master Nov 16, 2020
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

Sorry, @Fidget-Spinner, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 384b7a4bd988986bca227c7e85c32d766da74708 3.9

@miss-islington miss-islington self-assigned this Nov 16, 2020
@bedevere-bot

Copy link
Copy Markdown

GH-23309 is a backport of this pull request to the 3.9 branch.

@Fidget-Spinner
Fidget-Spinner deleted the ga-weakref branch November 16, 2020 05:31
miss-islington pushed a commit that referenced this pull request Nov 17, 2020
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
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.

7 participants