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

include offending line information for template encoding issues #334

Closed
wants to merge 2 commits into from

Conversation

jvanasco
Copy link
Member

Initial idea to handle #333 .

This caught me when we had to backport some new templates written for staging (Py3) to production (Py2). It should catch similar encoding issues on PY3 as well.

@zzzeek zzzeek requested a review from sqla-tester July 28, 2021 18:11
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 27cb5ee of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

New Gerrit review created for change 27cb5ee: https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/2987

# line but does not show the exact line.
try:
for idx in range(0, len(self.data)):
_content = self.delim.join(list(self.data)[0:idx+1])
Copy link
Member

Choose a reason for hiding this comment

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

you can do list(self.data) before the loop.

Also why not using a binary search?

Copy link
Member

Choose a reason for hiding this comment

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

Also why not using a binary search?

this is probably overkill for what this code aims to do, so nevermind that :)

"Exception raised on template line %s:" % (idx),
exc
)
raise
Copy link
Member

Choose a reason for hiding this comment

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

this is a leftover

for idx in range(0, len(self.data)):
_content = self.delim.join(list(self.data)[0:idx+1])
# this should never happen
return _content
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it could instead do nothing here and raise the original exception outside the inner try/except

@sqla-tester
Copy link
Collaborator

mike bayer (zzzeek) wrote:

moved to main

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/2987

@sqla-tester
Copy link
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/2987 has been abandoned. That means that at least for the moment I need to close this pull request. Sorry it didn't work out :(

@zzzeek zzzeek reopened this Oct 11, 2021
@bourke bourke added this to Backlog in Mako prioritization Nov 17, 2021
@bourke bourke moved this from Backlog to Active in Mako prioritization Nov 17, 2021
@bourke
Copy link
Member

bourke commented Dec 8, 2021

At @jvanasco 's suggestion, I'm closing this in favour of a Python 3-only approach.

@bourke bourke closed this Dec 8, 2021
Mako prioritization automation moved this from Active to Done Dec 8, 2021
@sqla-tester
Copy link
Collaborator

Michael Bourke (bourke) wrote:

Abandoned in favor of #3406

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/3142

@sqla-tester
Copy link
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/3142 has been abandoned. That means that at least for the moment I need to close this pull request. Sorry it didn't work out :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants