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

updated the wazimap version 1.1.1 #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kurund
Copy link

@kurund kurund commented Sep 21, 2018

No description provided.

@deshetti
Copy link
Member

@kurund Can you please add comments in terms of what features are added.

@kurund
Copy link
Author

kurund commented Sep 24, 2018

Copy link
Contributor

@mthipparthi mthipparthi left a comment

Choose a reason for hiding this comment

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

INSTALLEDAPPS directly added from wazimap. Just wondering why should be moving from wazimap?

DEBUG = config('DJANGO_DEBUG', default=True, cast=bool)
TEMPLATE_DEBUG = DEBUG

SECRET_KEY = 'wt08^t@ugzs4sw*qn=c*=$d+jgkqkkp4$0z98j-k5s!o2um$(n'
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, and in this case in particular, the secrets are read from config file in production.Having Secretkey in settings is not recpmmend.I used decouple libarary to read it from config file.

Choose a reason for hiding this comment

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

I have used decouple package for the secret key.

# DJANGO_SETTINGS_MODULE = config('DJANGO_SETTINGS_MODULE')

# install this app before Wazimap
INSTALLED_APPS = ['janaganana', 'django.contrib.sitemaps'] + INSTALLED_APPS
INSTALLED_APPS = [
'django.contrib.admin',
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering why we should be hardcodinh insattledapps when we can directly import from wazimap and add it here as that is future proof.

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@mthipparthi mthipparthi left a comment

Choose a reason for hiding this comment

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

INSTALLEDAPPS directly added from wazimap. Just wondering why should be moving from wazimap?

@kurund
Copy link
Author

kurund commented Nov 22, 2018

@mthipparthi any update on this?

Copy link
Contributor

@mthipparthi mthipparthi left a comment

Choose a reason for hiding this comment

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

My apologies for delay in review. couple of observations.

  1. SECRET is newly introduced in place of DJANGO_SECRET_KEY, deployment is failing.should hav been properly defaulted.
  2. Looks like there is some issue with changed table structure. I am not able to ru locally., getting below erros
    '''
    ci-app_1 | Unhandled exception in thread started by <function wrapper at 0x7f1a97b2f578>
    ci-app_1 | Traceback (most recent call last):
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py", line 226, in wrapper
    ci-app_1 | fn(args, **kwargs)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/django/core/management/commands/runserver.py", line 109, in inner_run
    ci-app_1 | autoreload.raise_last_exception()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py", line 249, in raise_last_exception
    ci-app_1 | six.reraise(
    _exception)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py", line 226, in wrapper
    ci-app_1 | fn(*args, **kwargs)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/django/init.py", line 18, in setup
    ci-app_1 | apps.populate(settings.INSTALLED_APPS)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/django/apps/registry.py", line 115, in populate
    ci-app_1 | app_config.ready()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/apps.py", line 15, in ready
    ci-app_1 | self.load_tables()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/apps.py", line 35, in load_tables
    ci-app_1 | import_module(app.name + '.tables')
    ci-app_1 | File "/usr/lib/python2.7/importlib/init.py", line 37, in import_module
    ci-app_1 | import(name)
    ci-app_1 | File "/usr/src/app/janaganana/tables.py", line 6, in
    ci-app_1 | FieldTable(['rural population'], universe='Population')
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/data/tables.py", line 393, in init
    ci-app_1 | db_table=db_table, **kwargs)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/data/tables.py", line 115, in init
    ci-app_1 | self.setup_columns()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/data/tables.py", line 414, in setup_columns
    ci-app_1 | self.build_models()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/data/tables.py", line 402, in build_models
    ci-app_1 | self.model = self._build_model_from_fields(self.fields, self.db_table, value_type=self.value_type)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/wazimap/data/tables.py", line 591, in _build_model_from_fields
    ci-app_1 | Model.table.create(session.get_bind(), checkfirst=True)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/sql/schema.py", line 818, in create
    ci-app_1 | bind._run_visitor(ddl.SchemaGenerator, self, checkfirst=checkfirst)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2010, in _run_visitor
    ci-app_1 | with self._optional_conn_ctx_manager(connection) as conn:
    ci-app_1 | File "/usr/lib/python2.7/contextlib.py", line 17, in enter
    ci-app_1 | return self.gen.next()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2002, in _optional_conn_ctx_manager
    ci-app_1 | with self.contextual_connect() as conn:
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2192, in contextual_connect
    ci-app_1 | self._wrap_pool_connect(self.pool.connect, None),
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2232, in _wrap_pool_connect
    ci-app_1 | e, dialect, self
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 1528, in _handle_dbapi_exception_noconnection
    ci-app_1 | util.raise_from_cause(sqlalchemy_exception, exc_info)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/util/compat.py", line 296, in raise_from_cause
    ci-app_1 | reraise(type(exception), exception, tb=exc_tb, cause=cause)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2228, in _wrap_pool_connect
    ci-app_1 | return fn()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 425, in connect
    ci-app_1 | return _ConnectionFairy._checkout(self)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 822, in _checkout
    ci-app_1 | fairy = _ConnectionRecord.checkout(pool)
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 554, in checkout
    ci-app_1 | rec = pool._do_get()
    ci-app_1 | File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 1250, in _do_get
    '''
    I would greatly appreciate if you can fix on your end. or please let me know if i am missing anything.
    I promise i would be prompt in review this time.

Sorry i am not able to approve this PR as i am not able to run locally.

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.

4 participants