Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
bpo-28806: Improve the netrc library#127
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
dmerejkowsky commented Feb 18, 2017
@zhangyangyu Nice work! I also wrote a tiny patch for |
zhangyangyu commented Feb 20, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@dmerejkowsky I wrote this patch months ago, so I have to spend some time picking it up to see if there is any possibility. Happy to see someone also interested in improving netrc. |
dmerejkowsky commented Feb 20, 2017
OK, then. Hopefully our PRs won't conflict with each other, so this not a big deal. Keep up the good work! |
…tor and traceback objects __setstate__ must accept the state returned by __reduce__. This was not the case for generator and trace-back objects. This commit fixes this. The next commit (merge of issue python#127) adds the relevant test cases. Additionally amends changelog.txt. https://bitbucket.org/stackless-dev/stackless/issues/107
…ling tests. This change is based on f5f98595c6cc63b from 2.7-slp, but it is heavily modified for Python 3. - Pickle 'callable-iterator' objects correctly. Previously the unpickled object had the type '_stackless._wrap.callable-iterator'. - Fix pickling of 'method-wrapper' objects. Previously pickling them caused a SystemError exception. - Fix copy.copy() for 'callable-iterator', 'method', 'dict_keys', 'dict_values' and 'dict_items' objects. Previously the copied object had the type '_stackless._wrap....'. - Fix Stackless pickling tests. The method StacklessTestCase.dumps() didn't pass the pickle protocol to the pickler. - Remove dead code in prickelpit.c. The code was used in older Stackless versions.
…iterator' Disable the Stackless specific code for pickling 'iterator' and 'callable_iterator' objects. C-Python 3.3 already pickles them. Add tests to ensure, that Stackless can still unpickle old pickles. https://bitbucket.org/stackless-dev/stackless/issues/127
| account=lexer.get_token() | ||
| eliftt=='password': | ||
| ifos.name=='posix'anddefault_netrc: | ||
| ifos.name=='posix'anddefault_netrcandlogin!="anonymous": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes that login has already been parsed, but the netrc format does not require providing login token before password token. It would be better to perform this check on storing parsed data into self.hosts[entryname], when all machine-related data is parsed and available.
auvipy left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also please re base
| .. versionchanged:: 3.4 Added the POSIX permission check. | ||
| .. versionchanged:: 3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can retarget it for python 3.9
serhiy-storchaka left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, but it may be worth to backport these changes to maintained versions. It fixes several bugs in the .netrc file parsing.
| .. versionchanged:: 3.4 Added the POSIX permission check. | ||
| .. versionchanged:: 3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.8
| continue | ||
| ifch=="\"": | ||
| forchinfiter: | ||
| ifch!="\"": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would revert this condition.
ifch=='"': returntokenAnd removed continue.
| self.lineno=1 | ||
| self.instream=fp | ||
| self.whitespace="\n\t\r " | ||
| self._stack= [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a stack, it is a queue. I suggest to rename it to pushback, as in shlex.lexer.
| forchinfiter: | ||
| ifchinself.whitespace: | ||
| continue | ||
| ifch=="\"": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'"'?
| ch=self._read_char() | ||
| token+=ch | ||
| forchinfiter: | ||
| ifchnotinself.whitespace: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as above.
| entryname='default' | ||
| eliftt=='macdef':# Just skip to end of macdefs | ||
| eliftt=='macdef': | ||
| entryname=lexer.get_token() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to read and ignore the rest of the line after the macro name.
| raiseNetrcParseError( | ||
| "Macro definition missing null line terminator.", | ||
| file, lexer.lineno) | ||
| ifline=='\n': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment.
| forhostinself.hosts.keys(): | ||
| attrs=self.hosts[host] | ||
| rep=rep+"machine "+host+"\n\tlogin "+repr(attrs[0])+"\n" | ||
| rep=rep+"machine "+host+"\n\tlogin "+attrs[0] +"\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we need a special function which escapes whitespaces and other special characters?
| ch=self._read_char() | ||
| token+=ch | ||
| forchinfiter: | ||
| ifchnotinself.whitespace: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to add a flag for denoting whether the terminating whitespace was '\n'. It will be helpful for comments and macdef.
csabella commented May 20, 2019
@zhangyangyu, would you be able to address Serhiy's review from October? Thanks! |
zhangyangyu commented May 20, 2019
@csabella I am busy in my job now and this issue has been a long time even myself needs to reacquaint with it. But October seems not that hard to me. Is 3.8 going to release on October? |
csabella commented May 31, 2019
@zhangyangyu, sorry, I meant that Serhiy had left a comment last October, not that this needs to be looked at before the next October. The cutoff for 3.8 is this coming Monday (2019-06-03). I hope you still find time to look at it in the next few months. Thanks! |
eamanu commented Sep 18, 2019
Hello I can continue this PR if your consider good. Cc @csabella |
eamanu commented Jun 21, 2020
Hi @auvipy@csabella@zhangyangyu There is some plan with this PR? What's your opinion? |
HighnessAtharva commented May 24, 2021
So should this PR be closed? |
nanjekyejoannah commented May 24, 2021
Closing this PR since another PR #26330 continues and addresses comments on this thread. All reviews should move to the new PR. |
No description provided.