c2corg / v6_ui

UI for c2c.org v6
GNU Affero General Public License v3.0
7 stars 12 forks source link

Add support for slacklining #1543

Closed tsauerwein closed 7 years ago

tsauerwein commented 7 years ago

Idea Topos for slacklines are treated as routes with the activity slackline. They are either associated to existing waypoints (e.g. summits, waterfalls, lakes, canyons, …) or to waypoints of the new type slackline_spot. A slackline spot, similar to a climbing spot, is an area where one or more slacklines can be rigged. An example for a slackline spot could be a park or the area of a highline festival with multiple lines.

Required changes

tsauerwein commented 7 years ago

Importing the Highline Database into my local instance took a bit longer than originally thought, but now we have 170 highline/waterline topos (and also 47 new waypoints) in a format to make the import. :)

I still have to download the images that are used in the descriptions (right now there is simply the URL to the image). And I also have to prepare a script which gets the documents from my local instance, and pushes them to integration/production.

I was planning to use the normal API to make the import. It would probably make sense to use the C2C Account for that. We should just make sure, that the imported documents do not appear in the feed (I can adapt the API). @asaunier, what do you think?

screenshot from 2017-05-07 10-27-29

screencapture-localhost-6554-routes-220-fr-cascade-des-thermes-highline-1494141877926

asaunier commented 7 years ago

Importing the Highline Database into my local instance took a bit longer than originally thought

Does it imply to do something special while the migration is running? Backuping and putting the site in maintenance mode?

I was planning to use the normal API to make the import. It would probably make sense to use the C2C Account for that.

What about creating and using a "Highline Database" instead? This way the origin of the imported documents would be more explicit. Would Julien be OK with this?

We should just make sure, that the imported documents do not appear in the feed (I can adapt the API).

I am not sure this import should be excluded from the feed. I admit this would be a bit invasive but at least it would give some visibility to this new activity and to all the work done to add it. Of course the moment the import would be done at should be chosen carefully to avoid upsetting people looking for info about their next WE :P And it should be done in coordination with the communication team. @desnoes @stef74 What do you think?

asaunier commented 7 years ago

I guess we'd better merge your PRs only when we're about to migrate the Highline Database data and make the new activity live.

tsauerwein commented 7 years ago

I guess we'd better merge your PRs only when we're about to migrate the Highline Database data and make the new activity live.

Yes, that was also my plan. But you can already review now. :)

tsauerwein commented 7 years ago

Does it imply to do something special while the migration is running? Backuping and putting the site in maintenance mode?

As the normal API is used, there is no need for maintenance mode. But making a database backup before doing the Alembic migration and the import is a good idea.

What about creating and using a "Highline Database" instead? This way the origin of the imported documents would be more explicit.

I gave credits in route_history, but using a distinct user could also be an option. At least no one could remove the credits it in that case.

I am not sure this import should be excluded from the feed. I admit this would be a bit invasive but at least it would give some visibility to this new activity and to all the work done to add it. Of course the moment the import would be done at should be chosen carefully to avoid upsetting people looking for info about their next WE :P And it should be done in coordination with the communication team.

If >200 entries for imported documents appear in the feed, it will be really a bit noisy. ;) I think there are other ways to inform about the new activity. And hopefully new highline topos by contributors will show up after the import (at least I have about dozen topos to add..).

asaunier commented 7 years ago

As the normal API is used, there is no need for maintenance mode. But making a database backup before doing the Alembic migration and the import is a good idea.

Sure. I always do a backup before running Alembic. How long does the slacklining import take?

If >200 entries for imported documents appear in the feed, it will be really a bit noisy. ;) I think there are other ways to inform about the new activity. And hopefully new highline topos by contributors will show up after the import (at least I have about dozen topos to add..).

OK

But you can already review now. :)

Yep. In my TODO :)

tsauerwein commented 7 years ago

How long does the slacklining import take?

Not sure. Uploading the images will take a moment, and I think my internet connection will be the limiting factor here.. ;) But creating the documents should be relatively fast.

stef74 commented 7 years ago

@tsauerwein We have plan to update demo tomorrow (afternoon ?) It is ready for merge ? The import can be done after ? Thank for the job !

tsauerwein commented 7 years ago

Yes, I think it would be good if the association had a version to test. And I could also test the migration with the full database this weekend.

@asaunier would you also be ok with merging now?

asaunier commented 7 years ago

would you also be ok with merging now?

@tsauerwein Sure!

tsauerwein commented 7 years ago

Great! @asaunier, if you deploy the demo, please remember to run the ES migration script (see https://github.com/c2corg/v6_api/tree/master/es_migration).

To test the migration, I would need an account. Should we create a new "Highline Database" account like discussed above? The user id of this account should be set in the config (see https://github.com/c2corg/v6_api/blob/master/config/default#L90).

asaunier commented 7 years ago

if you deploy the demo, please remember to run the ES migration script

Thanks @tsauerwein for pointing this, I had not realized this action was needed.

To test the migration, I would need an account. Should we create a new "Highline Database" account like discussed above? The user id of this account should be set in the config

Up to you. What do you think is the best option?

The easiest is probably to use the C2C Association account (id 2).

If we create an "Highline Database" account - which is probably better to identify the source of the imported data - we need to:

If an account must be created, it's pretty sure the user id will not be the same in the demo and in the prod. Then how could we change https://github.com/c2corg/v6_api/blob/master/config/default#L90 ?

What do you recommend?

Concerning the new slacklining-related strings, I have translated them (French only!) in Transifex but I am not a specialist. I hope my translations are OK: capture du 2017-05-12 11-04-36

We plan to do a release today, so we have to fix this user id question and important translations before doing so.

tsauerwein commented 7 years ago

For the demo, we could simply use the C2C-Account. But then for prod we could create a separate account (me or the association) and use this account only for the migration (nothing else).

asaunier commented 7 years ago

OK then:

The only problem I see is that we won't be able to test that the import is not shown in the demo feed. Unless we don't change https://github.com/c2corg/v6_api/blob/master/config/default#L90 now and postpone the import after the following release?

tsauerwein commented 7 years ago

The only problem I see is that we won't be able to test that the import is not shown in the demo feed.

Not sure I understand you right. By default the C2C account is ignored, so this should be the case if you update the demo.

Also, can't this setting be set in a Docker environment variable (the same way as for example the database connection string)? There should be no need to make a PR just to change this config.

Thanks for the French translation, your translations sound good! Could you please add me to the English translation team?

asaunier commented 7 years ago

Not sure I understand you right. By default the C2C account is ignored, so this should be the case if you update the demo.

Do you mean that whatever the value of https://github.com/c2corg/v6_api/blob/master/config/default#L90 (for instance if set to the id of user "Highline Database") actions done with the C2C account are ignored?

My concern was that if only one account can be ignored, we could not use either the C2C account in the demo and the Highline Database account in the prod to import the data and have the import ignored in the feed of both instances.

Also, can't this setting be set in a Docker environment variable (the same way as for example the database connection string)? There should be no need to make a PR just to change this config.

That would make sense indeed. But I have no clue how this is done using the docker compose config.

Could you please add me to the English translation team?

You should have received an invitation. It seems you are already in the german translation team.

asaunier commented 7 years ago

Concerning the translation strings, I have noticed there were some (nearly) duplicates introduced by this PR. For instance "Height" or "Length" ("height" or "length" were already available) or "slackline" / "Slackline". I think the translations are capitalized anyway, then are they really needed?

tsauerwein commented 7 years ago

Do you mean that whatever the value of https://github.com/c2corg/v6_api/blob/master/config/default#L90 (for instance if set to the id of user "Highline Database") actions done with the C2C account are ignored?

No, only the actions done by the account set in the configuration are ignored.

That would make sense indeed. But I have no clue how this is done using the docker compose config.

I can't remember how this was set up. But Arnauld and Marc will know how to configure it.

I've prepared the import scripts, from my side it would be ready to test on the demo.

asaunier commented 7 years ago

I can't remember how this was set up. But Arnauld and Marc will know how to configure it.

I have never done it but I have looked at the demo's docker-compose.yml file and for UI and API parts it has an environment section defining a set of environment variables:

  api:
    image: 'c2corg/v6_api:v6.1.3'
    environment:
      ui_url: 'https://www.demov6.camptocamp.org'
      image_backend_url: 'http://images.demov6.camptocamp.org'
      discourse_url: 'http://forum.demov6.camptocamp.org'
      discourse_public_url: 'https://forum.demov6.camptocamp.org'
      image_url: 'https://sos.exo.io/c2corg_demov6_active/'
      ...

So I guess we should simply add an environment variable entry for your feed_admin_user_account and this will override https://github.com/c2corg/v6_api/blob/master/config/default#L90 @mfournier @arnaud-morvan Do you confirm?

asaunier commented 7 years ago

@tsauerwein I suggest that you give me your user id if you have an account in the demo and I will use this id in the docker-compose.yml and you'll make the import with your own account. This way we can test that the config is correctly taken into account to ignore the import.

By the way for the following release I'll have to remove the config from the docker compose file so that future contributions by the Highline Database account are normally advertised in the home feed.

fbunoz commented 7 years ago

for prod we could create a separate account (me or the association) and use this account only for the migration (nothing else).

We can use the C2C assoce account, but add a specific comment on the creation (there is a text field to comment a creation or a change of a doc). The comment can contain the source url. On the other hand, using a specific account allow to describe the source and the history of this project, using the description field of the account. But these infos will also be published on the article which will describe the c2c highline database (with the help on how to add a highline, etc).

tsauerwein commented 7 years ago

I suggest that you give me your user id if you have an account in the demo

I created an account on the demo for the import: http://www.demov6.camptocamp.org/profiles/828675 Could you please make this account a moderator, so that I can delete documents in case something goes wrong during the import (update users.user set moderator = true where id = 828675;)? Thanks!

We can use the C2C assoce account, but add a specific comment on the creation ... The comment can contain the source url.

You can't give a comment when creating a document, only when updating. I left a comment in field route_history ("Imported from the Highline Database"). I could make this a link pointing to the source URL.

On the other hand, using a specific account allow to describe the source and the history of this project, using the description field of the account.

Yes, that is a good idea!

asaunier commented 7 years ago

@tsauerwein I have just updated the demo to releases UI v6.1.8 and API v6.1.4 (including the slacklining changes). I have run successfully the alembic script as well as the ES migration script mentioned in https://github.com/c2corg/v6_api/tree/master/es_migration

root@demo:~/docker-stuff/composition/demov6# docker-compose run --rm api .build/venv/bin/python es_migration/2017-03-29_slackline.py production.ini
run-parts: executing /docker-entrypoint.d/10-env-replace.sh
scripts/env_replace < common.ini.in > common.ini
chmod --reference common.ini.in common.ini
scripts/env_replace < production.ini.in > production.ini
chmod --reference production.ini.in production.ini
scripts/env_replace < MANIFEST.in > MANIFEST
chmod --reference MANIFEST.in MANIFEST
scripts/env_replace < development.ini.in > development.ini
chmod --reference development.ini.in development.ini
scripts/env_replace < alembic.ini.in > alembic.ini
chmod --reference alembic.ini.in alembic.ini
scripts/env_replace < test.ini.in > test.ini
chmod --reference test.ini.in test.ini
scripts/env_replace < c2corg_api/scripts/loadtests/loadtests.ini.in > c2corg_api/scripts/loadtests/loadtests.ini
chmod --reference c2corg_api/scripts/loadtests/loadtests.ini.in c2corg_api/scripts/loadtests/loadtests.ini
scripts/env_replace < c2corg_api/scripts/migration/migration.ini.in > c2corg_api/scripts/migration/migration.ini
chmod --reference c2corg_api/scripts/migration/migration.ini.in c2corg_api/scripts/migration/migration.ini
scripts/env_replace < Dockerfile.in > Dockerfile
chmod --reference Dockerfile.in Dockerfile
scripts/env_replace < apache/app-c2corg_api.wsgi.in > apache/app-c2corg_api.wsgi
chmod --reference apache/app-c2corg_api.wsgi.in apache/app-c2corg_api.wsgi
scripts/env_replace < apache/wsgi.conf.in > apache/wsgi.conf
chmod --reference apache/wsgi.conf.in apache/wsgi.conf
ElasticSearch version: 2.3.5
Field "slackline_type" created

I have also added feed_admin_user_account: 828675 to the api: environment section of the docker-compose.yml before running the upgrade. I have also made this user a moderator in the demo.

desnoes commented 7 years ago

I have created a new topo (http://www.demov6.camptocamp.org/routes/828876/fr/mont-blanc-essai-slackline) and an outing associated to this topo (http://www.demov6.camptocamp.org/outings/828897/fr/mont-blanc-essai-slackline). I observe the outing in the list of outing = > OK I don't see the topo when i select the activity 'slacklining' in the guidebook => NOK

tsauerwein commented 7 years ago

I have just updated the demo to releases UI v6.1.8 and API v6.1.4 (including the slacklining changes).

Great, thanks a lot Alex! I've just finished uploading the topos: http://www.demov6.camptocamp.org/routes#act=slacklining

Looks good so far! And the changes are not shown in the feed. :)

I don't see the topo when i select the activity 'slacklining' in the guidebook => NOK

This must have been related to the fact that the route geometry was far away from the waypoint. I changed the geometry closer to the Mont Blanc and now the route is listed: http://www.demov6.camptocamp.org/routes#act=slacklining&bbox=757620%252C5740791%252C773595%252C5763722

desnoes commented 7 years ago

@tsauerwein : ok, i didn't know there was that kind of check.

tsauerwein commented 7 years ago

@asaunier, when are you planning to deploy to production? I would try to run the import shortly after that.

//edit: The import account for production would be 884895.

asaunier commented 7 years ago

when are you planning to deploy to production?

@tsauerwein I don't know, most likely today or tomorrow. Would it be OK for you? @stef74 @desnoes When do you think is the best moment to do the upgrade?

stef74 commented 7 years ago

Between 10 - 16h ( people work :-p)

asaunier commented 7 years ago

Between 10 - 16h

Today? OK for you @tsauerwein ?

stef74 commented 7 years ago

Better today ... but tommorow possible ( but not fan to upgrade friday)

tsauerwein commented 7 years ago

Today? OK for you @tsauerwein ?

I will only be able to run the import this evening, but otherwise ok for me. Could you again make the import account a moderator (just in case)? Thanks!

asaunier commented 7 years ago

@tsauerwein I will do the upgrade at the end of the afternoon.

Could you again make the import account a moderator (just in case)

Done. Please let me know when you no longer need the moderator permissions for this user, I prefer limiting those permissions to actual moderators.

asaunier commented 7 years ago

@tsauerwein @stef74

I have just tried doing the upgrade in prod but I have finally aborted it because I got weird errors while running the alembic script used to make the changes in the database:

INFO  [alembic.runtime.migration] Running upgrade 91b1beed9a1c -> bacd59c5806a, Add slacklining
Traceback (most recent call last):
  File "/var/www/.build/venv/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 1182, in _execute_context
    context)
  File "/var/www/.build/venv/lib/python3.4/site-packages/sqlalchemy/engine/default.py", line 462, in do_execute
    cursor.execute(statement, parameters)
psycopg2.extensions.TransactionRollbackError: deadlock detected
DETAIL:  Process 12716 waits for AccessExclusiveLock on relation 564267 of database 20497; blocked by process 26005.
Process 26005 waits for AccessShareLock on relation 564237 of database 20497; blocked by process 12716.
HINT:  See server log for query details.

Perhaps related to a problem with an error I got whille trying to run the DB backup before the update, following the steps at https://github.com/c2corg/v6_api/wiki/Backup-the-prod-database

docker-compose exec postgresql bash

has failed with the following error:

No container found for postgresql_1

The API and UI are still working as expected but there is probably something weird to investigate before running the upgrade again.

tsauerwein commented 7 years ago

Ok, weird. Power is off here, so I probably couldn't have done the import this evening anyway (TIA).

fbunoz commented 7 years ago

For the data import, it should be better to set the first cross in "history" field, and the link to the highline data base in the "external ressouce" field, instead of setting both in "history" field. Ex: http://www.demov6.camptocamp.org/routes/829150/fr/panta-de-margalef-ingrid-est-ce-que-tu-baises

tsauerwein commented 7 years ago

For the data import, it should be better to set the first cross in "history" field, and the link to the highline data base in the "external ressouce" field, instead of setting both in "history" field.

Ok, I restructured the fields. Note that this change is not visible on the demo.

asaunier commented 7 years ago

I have tried to run the alembic script on the prod database again this morning but I still get a deadlock, preventing the DB to be upgraded:

...
INFO  [alembic.runtime.migration] Running upgrade 91b1beed9a1c -> bacd59c5806a, Add slacklining
Traceback (most recent call last):
  File "/var/www/.build/venv/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 1182, in _execute_context
    context)
  File "/var/www/.build/venv/lib/python3.4/site-packages/sqlalchemy/engine/default.py", line 462, in do_execute
    cursor.execute(statement, parameters)
psycopg2.extensions.TransactionRollbackError: deadlock detected
...
sqlalchemy.exc.OperationalError: (psycopg2.extensions.TransactionRollbackError) deadlock detected
DETAIL:  Process 10170 waits for AccessExclusiveLock on relation 564112 of database 20497; blocked by process 10116.
Process 10116 waits for AccessShareLock on relation 564337 of database 20497; blocked by process 10170.
HINT:  See server log for query details.
 [SQL: 'ALTER TABLE guidebook.articles ALTER COLUMN activities TYPE guidebook._activity_type[] USING activities::text[]::guidebook._activity_type[]']
tsauerwein commented 7 years ago

I might be necessary to have a small down-time. The migration script changes the column types, and as long as there are requests, Postgres might not be able to do the change.

asaunier commented 7 years ago

OK thanks. I guess this means I have to put the 3 api instance in maintenance mode:

docker-compose exec api touch maintenance_mode.txt

and then only run the alembic script while upgrading the first instance. Correct?

asaunier commented 7 years ago

I have to put the 3 api instance in maintenance mode

I have tried to do so: all 3 api instances in maintenance mode, pulled/downed them and run the alembic script on the first instance => UI returned a 503 error whereas the alembic script remained frozen on the slacklining upgrade for more than 10 minutes. I have finally interrupted the script and restarted the api instances with the former image. Still failing to upgrade then. @arnaud-morvan Would you have an idea?

tsauerwein commented 7 years ago

Sorry, that this causing so much trouble. You could check if there is a blocked process with select * FROM pg_stat_activity:

https://stackoverflow.com/questions/5408156/how-to-drop-a-postgresql-database-if-there-are-active-connections-to-it/19884946

asaunier commented 7 years ago

@tsauerwein Thanks for the hint! The prod upgrade is postponed to next week since a long week end starts tomorrow.

asaunier commented 7 years ago

@tsauerwein The prod has been upgraded with your changes about slacklining as well as config feed_admin_user_account: 884895.

tsauerwein commented 7 years ago

Great, thanks! I started the import. My connection is a bit slow, so it might take a while. I will let you know when I am done.

tsauerwein commented 7 years ago

Done: https://www.camptocamp.org/routes#act=slacklining

:)

asaunier commented 7 years ago

Yoohoo! Thanks :) I will let the association know. Thanks again for the great work!

asaunier commented 7 years ago

Someone has already added a new line yesterday evening: https://www.camptocamp.org/routes/890563/fr/brevent-attention-ou-tu-saute

But I don't see it in the list of slacklining routes: https://www.camptocamp.org/routes#act=slacklining&bbox=751247%252C5752451%252C771656%252C5785090

@tsauerwein A problem with the ES indexing? Do we use ES when filtering on an activity?

I remember @mfournier had switched the syncer off yesterday when we made the prod upgrade but I think he had switched it on again. We have also run the ES migration script at https://github.com/c2corg/v6_api/tree/master/es_migration (with production.ini).

tsauerwein commented 7 years ago

But I don't see it in the list of slacklining routes:

It's shown now. Maybe the syncer had a hickup. :S