search

nudj / nudj-backend

Nudj - Backend (Archive)
0 stars 0 forks source link

linkedin_token column in users database table #6

Closed shtukas closed 8 years ago

shtukas commented 8 years ago

I removed the LinkedIn related code from the api source, but the users database table still carries the column. Not harmful but could be removed to keep things tidy.

richardbuckle commented 8 years ago

Presumably we can only remove it after deploying the revised API code?

shtukas commented 8 years ago

Oh yes. Obviously. Sorry I should have made that clear.

That being said, and this is really for argument sake, in this case it would not matter.

richardbuckle commented 8 years ago

OK, please go ahead and remove it when it is safe to do so.

shtukas commented 8 years ago

Done.

richardbuckle commented 8 years ago

Ah. This turns out not to be as safe as we thought:

"API error: SQLSTATE[42S22]: Column not found: 1054 Unknown column \'linkedin_token\' in \'field list\' (SQL: select `id`, `status`, `facebook_token`, `linkedin_token` from `users` where `users`.`id` = 2 limit 1)"

This change appears to have been pushed to production and unfortunately it is breaking the version we just shipped to the App Store. We need to roll back this change ASAP I'm afraid.

New policy: no changes are to be pushed to the production server until they have first passed testing on the dev server.

shtukas commented 8 years ago

New policy: no changes are to be pushed to the production server until they have first passed testing on the dev server.

I actually agree with that 1000% (I am all too aware of the crucial importance of the backend) which is the reason why I haven't activated the TLS switch for the production chat (I prefer giving it to you on the new chat servers I am building).

Otherwise, I haven't made a code change today (or the past few days actually), and the dev and prod code are the exact same. What I did though was to remove the column in the database (which was long overdue), I was just cleaning Github issues before tomorrow's meeting. I did this before and after running the unit tests.

Now, the fact that this has broken something is incredibly frustrating to me, as you can imagine... So this is what I am going to do

  1. Put the column back in the prod database. Done. Please tell me if the problem still happens.
  2. Investigate why this happened. I suspect that there might be something that the iOS client is doing that I am not fully aware of. I will investigate (That's a perfect CSI episode for me ^_^)

Cheers,

shtukas commented 8 years ago

Please tell me if the problem still happens.

I want to try something. Please don't do anything before I am done. I just need few minutes.

richardbuckle commented 8 years ago

Appears to be fixed (dev server).

shtukas commented 8 years ago

Done. Here is the situation:

i. Few seconds ago, I removed the database column that I had added back in few minutes ago. So as of now the column is not there. ii. I have just pushed version Version 1.011 (2016-03-06).

I did this because I am less interested in solving the problem and more interested in solving it and also ensuring that I understand why it happened in the first place (so that I prevent it from happening).

I would like you to tell me if the problem still occurs. If has been fixed, then.... good, I guess. If it has not, been fixed, then given the modification I did I will be able to conclude that the iOS was the source of the problem.

... update me at your earliest convenience. Cheers,

shtukas commented 8 years ago

Appears to be fixed (dev server).

Awesome !

richardbuckle commented 8 years ago

And it remains fixed with your further modifications.

Do we have test coverage on the code in question?

shtukas commented 8 years ago

Do we have test coverage on the code in question?

Um..... the correct answer is no, but only because the update I just pushed was to remove code. I was doing some cleaning this week end for my own peace of mind and (by accident, or luck, or my own personal insight) it turned out to be the solution to the problem you reported.

By the way, I want to clarify something. In this moment, we have only one database used both by the production instance and the dev instance. This configuration was set up by me before we went live as it was the safest way to ensure that chat would work safely during the apple review. And obviously we are still running on it because I could not afford to break anything during the review process.

In order to (re)segregate between dev and prod (my highest priority right now), I need to release to you the two chat servers (one for prod, one for dev) that I am cooking [1]. Only when this will be done, I will be able to have two database and then two separate networks (prod api, prod database and prod chat, on one side, and dev api dev database and dev chat) on the other.

[1] This is taking time, because during all this I refuse to make changes to the existing chat server.

Anyway, I am glad it took me less time to fix your problem than writing those explanation posts :)

Cheers,