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

Avoid infinite recursion when renaming #3326 #3330

Merged
merged 1 commit into from
Apr 23, 2015

Conversation

rafatower
Copy link
Contributor

See the ticket for more info.

@Kartones please review

See the ticket for more info.

unless errored
if @user_table.layers.blank?
exception_to_raise = CartoDB::TableError.new("Attempt to rename table without layers #{qualified_table_name}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd either change to raise error (as we're not launching/raising an exception), or as you told me in person, directly remove this par of the if and let the code explode and/or be treated at upper layers if layers are empty.

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Kartones
Copy link
Contributor

👍 Apart from that exception_to_raise only used for notification and not actual raising

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@rafatower rafatower force-pushed the 3326-avoid-infinite-recursion-when-renaming branch from 2e1d70e to 21224cc Compare April 23, 2015 08:55
@rafatower
Copy link
Contributor Author

I tested 2e1d70e but in the end I prefer to keep the CartoDB::notify_exception so that it appends the user info to the trace. In case of weirdness it can greatly help us reconstruct what happened.

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

rafatower pushed a commit that referenced this pull request Apr 23, 2015
…hen-renaming

Avoid infinite recursion when renaming #3326
@rafatower rafatower merged commit 87a9dd7 into master Apr 23, 2015
@rafatower rafatower deleted the 3326-avoid-infinite-recursion-when-renaming branch April 23, 2015 09:09
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.

None yet

3 participants