Issue #80 #84

punkyoon · 2017-03-11T21:09:18Z

python3 deploy_helper.py실행 시, pip 설치부터 nginx 설치, DB migration 등 서버 배포에 필요한 모든 과정을 자동으로 실행. 사용자에게 입력 받는 부분은 OAuth 설정 관련한 것 이외에는 없음.

python3 undeploy_helper.py실행 시, 서비스 종료(redis&nginx) 및 프로세스 종료 실행. 서버 종료에 필요한 모든 과정을 자동으로 실행.

minhoryang

deploy_helper.py의 역할은 무엇인가요?
개발환경을 빠르게 구축하기 위함인가요?
undeploy_helper.py의 역할은 무엇인가요?
개인적인 deploy관리를 위함인가요?

minhoryang · 2017-03-12T11:14:37Z

deploy_helper.py

+except ImportError:
+    print("Installing pip...")
+    if platform == 'linux':
+        os.system('sudo apt-get install python3-pip')


pip를 설치만 하고, import를 진행하지 않습니다. Line 51에서 pip를 찾지 못할 예정입니다.

minhoryang · 2017-03-12T11:15:02Z

deploy_helper.py

+platform = sys.platform
+if platform == 'win32' or platform == 'win64':
+    print('Error: Cannot run in Windows..')
+    exit(0)


Error의 경우의 return값은 0이 아니어야 합니다. (공통)

minhoryang · 2017-03-12T11:17:13Z

deploy_helper.py

+
+# Check OS
+platform = sys.platform
+if platform == 'win32' or platform == 'win64':


if platform in ( 'win32', 'win64', ):

이런 식이 더 나을 것 같습니다.

minhoryang · 2017-03-12T11:18:14Z

deploy_helper.py

+import json
+
+
+def open_secret():


함수의 이름이 적절하지 않습니다.
create_secret_json?

minhoryang · 2017-03-12T11:32:11Z

deploy_helper.py

+        print('*** Please write your Server Domain (ex. example.com)')
+        domain = input('>')
+
+        result = json.load(f)


기존에 있는 값을 존중하나요? 무슨 의미의 json.load인가요? input과 어떤 상호작용을 하나요?

와우... SECRET_KEY가 보존되는군요...
https://github.com/dduk-ddak/coding-night-live/pull/84/files#diff-e39648c4368f8290aedea7d8cac3e743R10

그러면 이 함수는 secret_key_gen.py보다 늦게 실행되어야 겠네요.
secret.json이 없으면 에러가 나니까요.
그냥 합치면 안될까요?

python manage.py migrate가 SECRET_KEY를 필요로 하나요?

migrate 이전에 secret_key가 필요합니다.

python3 secret_key_gen.py 실행 시, secret_key만 명시되어있는 secret.json 생성

input으로 OAuth 관련 설정 입력 받아 secret.json에 추가해줌

python3 manage.py autodeploy에서 secret.json에 있는 내용을 토대로 OAuth 설정

migrate 이전에 SECRET_KEY가 필요한지를 현재 코드 상황에서는 알았습니다.
그런데 'migrate 과정에서 SECRET_KEY가 필요한가'라는 물음을 확인하지 못했을 뿐입니다.
django가 SECRET_KEY를 필요로 하다는걸 ImproperlyConfigured로 확인했습니다만,
settings.py에 정의되어 있는 SECRET_KEY를 조금 수정해서, 그 단계를 넘길 수 있다고 생각하고 있습니다.
정말 migrate단계에서 SECRET_KEY가 필요할까요?
secret_key_gen.py와 이 파일을 합칠 수 있지 않을까요?

autodeploy에서 migrate를 할 수 없나요?

SECRET_KEY=1 로 고정 후 python manage.py migrate를 진행하고 나서도,
python secret_key_gen.py, python manage.py cratesuperuserauto, python manage.py runserver를 통해서 admin페이지에 잘 접근이 가능함을 확인하였습니다.

minhoryang@a135e31

minhoryang · 2017-03-12T11:35:21Z

deploy_helper.py

+    for package in packages:
+        if package[0] == '#':
+            break
+        pip.main(['install', package])


pip.main(['install', '-r', 'requirements.txt'])

minhoryang · 2017-03-12T11:36:02Z

deploy_helper.py

+cmd = 'python3'
+
+# Check Python Version (3 or 2)
+if sys.version_info[0] == 2:


무슨 이유로 이 파일은 python3만 지원하는건가요?

@minhoryang django 2.0부터는 python2.7을 아예 지원하지 않습니다. 추후 django 버전 업을 대비하여 2버전에서는 아예 실행하지 못하도록 막아두었습니다.

@punkyoon 오! 장고도 드디어! 알겠습니다 :)

@punkyoon Django 2.0으로의 migration(?)이 바람직할려나 모르겠네요! 1.1x를 그대로 갖고 가는게 나을지도 모르고요

django 2.0에서의 channels 지원 여부에 따라 좀 갈릴 것 같네요. 일단 py2에서도 하게 하는게 맞는 것 같아요!

@juice500ml django-channels는 py3.5+를 지원한다는데, django 2.0이 channels를 지원할 계획인지 여부 말하시는거죠?
https://github.com/django/channels/blob/13472369ebf18d2925302aa1a407405b67a3e7c5/tox.ini

@minhoryang 넵넵 그쵸 사실 django-channels가 django 1.10에 들어오기로 했었는데 그게 무산됐거든요! django 2버전에서는 어떻게 될지 아직 미정 상태인거같아요

minhoryang · 2017-03-12T11:42:51Z

undeploy_helper.py

+    exit(0)
+
+# Delete files
+os.system('rm -rf pw.txt')


rm -rf로 기존에 있는 파일을 꼭 지워야 하나요?
open(, 'w')도 마찬가지입니다.

삭제하려면 pw.txt, secret.json, db.sqlite3의 경우 rm -f 만으로 하는게 바람직할 것 같습니다.
collected_static은 삭제해야하는지 약간은 의문이네요!

minhoryang · 2017-03-12T11:49:12Z

nginx/local_nginx.conf

@@ -6,21 +6,21 @@

 server {
  listen 80;
-  server_name localhost;
+  server_name coding-night-live.japaneast.cloudapp.azure.com;


이 파일을 django template로 뺼 순 없을까요?

http://stackoverflow.com/questions/32442893/render-django-template-from-command-line-without-settings

minhoryang@9355534
한번 해보았습니다!

minhoryang · 2017-03-12T11:54:16Z

코딩스타일, posix에러코드, 템플릿 추출, 설정보관 등등 여러가지 리뷰항목이 있는데요.

이 파일의 예상 사용자를 누구로 잡았는지가 궁금합니다.
(디버깅/테스트용인지, 개인 배포 자동화용인지, 외부인 배포 자동화용인지, ...)

minhoryang · 2017-03-12T11:59:37Z

coding_night_live/management/commands/autodeploy.py

+        default_site_1.name = domain
+        default_site_1.save()
+
+        new_social_app = SocialApp(


이 함수를 두번 호출하면, id=1인 SocialApp을 두번 생성하려 할겁니다.

이 함수를 두번 부를 일이 없을거라고 생각하지만,
undeploy_helper.py의 역할이 무엇이냐에 따라서, 다를 수도 있습니다.

id가 autoincrement 될테니 굳이 id=1로 안해도 되지 않나요?

Ref.
SocialApp은 django.db의 Model을 상속받고,
Model의 id는 serial NOT NULL PRIMARY KEY인데,
SERIAL은 BIGINT UNSIGNED NOT NULL AUTO_INCREMENT UNIQUE 입니다.

그렇게할까요? @punkyoon @juice500ml

minhoryang · 2017-03-12T11:59:56Z

coding_night_live/management/commands/autodeploy.py

+
+class Command(BaseCommand):
+    def system_check(self):
+        # win32 / win64 / linux(Ubuntu)


TODO인건가요?

minhoryang · 2017-03-12T12:01:09Z

deploy_helper.py

+        pip.main(['install', package])
+
+# DB Migration
+os.system('%s secret_key_gen.py' % cmd)


아래의 많은 os.system을 makefile로 옮기는건 어떻게 생각하시나요?

minhoryang@be68a4c

minhoryang · 2017-03-12T12:05:45Z

deploy_helper.py

+# nginx = subprocess.checkoutput('sudo find / -name nginx.conf', shell=True)
+
+# Server Deploy
+BASE_DIR = os.getcwd()


os.getcwd()는 현재 이 파일의 위치를 보장하지 않습니다.
os.path.dirname(os.path.abspath(__file__))를 사용하세요.

minhoryang · 2017-03-12T12:05:56Z

deploy_helper.py

+
+# Server Deploy
+BASE_DIR = os.getcwd()
+os.system('sudo rm -rf /etc/nginx/sites-enabled/local_nginx.conf')


이 파일의 백업본을 만들어주세요.

minhoryang · 2017-03-12T12:08:53Z

deploy_helper.py

+os.system('%s manage.py autodeploy' % cmd)
+
+# Server run
+os.system('redis-server &')


아래에 &을 붙은 명령들이 모두 background service로 남길 바라시나요?
모두 sighup을 무시하나요? 다른 서비스 매니저가 필요할 것 같은데요?
(ex. supervisord라던지)

juice500ml · 2017-03-12T14:01:38Z

coding_night_live/management/commands/createsuperuserauto.py

@@ -6,6 +6,11 @@ class Command(BaseCommand):

    def handle(self, *args, **options):
        password = get_random_string()
+
+        pwfile = open('pw.txt', 'w')


with open('pw.txt', 'w') as f: f.write(password)

가 더 나을 것 같아요!

juice500ml · 2017-03-12T14:08:05Z

#60 은 어떤 식으로 적용되었나요?

juice500ml · 2017-03-13T09:52:15Z

.gitignore

 # For hide SECRET_KEY
 secret.json
+pw.txt


@punkyoon @minhoryang secret.json과 pw.txt가 쪼개져있는게 약간 미묘해요. secret.json 안에 계정 비밀번호를 적어놔도 좋지 않을까 싶어요.

지금 secret.json이 DOMAIN도 가지고있어서 사실 애매모호해요.ㅎㅎ 이번 리뷰과정중에서도 이걸 짚고 넘어갈 수 있구, 아니면 다음에 해도 될 것 같아요.

…conf`

minhoryang · 2017-03-13T10:30:38Z

제가 nginx/local_nginx.conf 를 생성하는 코드를 짜보았는데요.
minhoryang@9355534

한번 리뷰해주시겠어요?

minhoryang · 2017-03-13T11:52:18Z

daphne가 killall로 죽지않아요 ㅠㅠ

minhoryang · 2017-03-13T12:04:24Z

제가 makefile를 조금 더 건드려봤는데, 매우 구질구질하네요
minhoryang@9c6cb68
만약 싫으시면 무시하시고,
be68a4c 만 revert 시키면 될 것 같아요.

punkyoon · 2017-03-13T15:11:02Z

daphne가 killall로 안죽다니..! 처음 접해보는 문제군요!

minhoryang · 2017-03-13T15:34:22Z

@punkyoon OS X 에서는 daphne가 python뒤에 붙어서 켜지더라구욧.

punkyoon · 2017-03-13T16:50:35Z

제가 너무 리눅스에 맞춰서 개발한 탓이네요..ㅠ_ㅠ

minhoryang · 2017-03-15T06:48:12Z

@punkyoon 아니에요아니에요!!!! 리눅스에서 잘 되면 다행인거죠! 무슨 포인트가 달랐는지 알아보고있어요!

Issue 80. nginxconfgenerator, makefile for MacOS

juice500ml · 2017-03-18T07:48:41Z

python locale도 잡아줘야 할까요? 뭔가 도를 넘은 것 같기도..

punkyoon · 2017-03-18T10:49:58Z

@juice500ml 아직은 안잡아줘도 될 거 같아요!

…rted as we expect

… 80 port on Mac OS

…d as we expected

Brand New Makefile

minhoryang · 2017-03-18T15:32:48Z

Makefile 테스트만 한번 더 해주시겠어요?

…ent for Twisted

minhoryang · 2017-03-19T09:16:24Z

제가 해보았습니다!

punkyoon added 18 commits March 8, 2017 04:51

createsuperuserauto command creates pw.txt file.

08852d4

modified .gitignore

eb3516b

create python script for auto deploy

247b7a8

added a autodeploy command

aee790f

developing autodeploy command...

95568e5

developting autodeploy command ... ; now can set oauth provider

aa5ac62

added a script file

d4bf728

developing script ..

cc5afce

developing script...

f1e9236

developing script...

67a5152

developing script.. but there's some error.. I will fix it soon

2a8403c

fixed a bug

baa8bfd

can deploy with auto.py

d7f0252

modified a .gitignore

fa7cdee

remove collected_static

7ab9f8d

developed deploy&undeploy helper

ba0d8ad

fixed flake8

c4a6bd9

change path

90ebf7e

punkyoon requested review from juice500ml and fuzzythecat March 11, 2017 21:12

punkyoon mentioned this pull request Mar 11, 2017

shell 작성 #80

Closed

minhoryang requested changes Mar 12, 2017

View reviewed changes

minhoryang reviewed Mar 12, 2017

View reviewed changes

juice500ml reviewed Mar 12, 2017

View reviewed changes

juice500ml reviewed Mar 13, 2017

View reviewed changes

[Issue #80] `python manage.py nginxconfgenerator > nginx/local_nginx.…

9355534

…conf`

구질구질한 Makefile

9c6cb68

juice500ml removed the request for review from fuzzythecat March 13, 2017 12:28

AilisObrian and others added 3 commits March 15, 2017 19:24

Merge remote-tracking branch 'minhoryang/issue_80' into issue_80

b713d03

Minor Feedbacks Apply

0ae5cf3

Merge pull request #87 from minhoryang/issue_80

fa8940e

Issue 80. nginxconfgenerator, makefile for MacOS

AilisObrian and others added 7 commits March 18, 2017 22:15

[Issue #84][BugFix] NGINX required sudoer for 80 port

79ec867

[Issue #84][BugFix] Latest Redis from Homebrew Kegs(cached) won't sta…

ae66ab6

…rted as we expect

[Issue #84] Purge makefile for deps

5d9632f

[Issue #84][BugFix][Related 79ec867] NGINX required sudoer rights for…

48226e1

… 80 port on Mac OS

[Issue #84][BugFix][Related 48226e1] sudo brew services won't worke…

a34f399

…d as we expected

[Issue #84][Readability Improvement] Divided Makefile

c173f85

Merge pull request #92 from minhoryang/issue_80

1ca9ed8

Brand New Makefile

minhoryang approved these changes Mar 18, 2017

View reviewed changes

Tiny typo fix at requirements.txt

c04bcb3

Daphne-Twisted Bug Resolved, so remove the version requirement statem…

7a01754

…ent for Twisted

minhoryang merged commit 02b36ac into master Mar 19, 2017

minhoryang deleted the issue_80 branch March 19, 2017 13:32

Issue #80 #84

Issue #80 #84

Conversation

punkyoon commented Mar 11, 2017

minhoryang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

punkyoon Mar 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minhoryang commented Mar 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juice500ml Mar 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minhoryang Mar 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juice500ml Mar 12, 2017 • edited Loading

Choose a reason for hiding this comment

juice500ml commented Mar 12, 2017

Choose a reason for hiding this comment

minhoryang Mar 13, 2017 • edited Loading

Choose a reason for hiding this comment

minhoryang commented Mar 13, 2017

minhoryang commented Mar 13, 2017

minhoryang commented Mar 13, 2017

punkyoon commented Mar 13, 2017

minhoryang commented Mar 13, 2017

punkyoon commented Mar 13, 2017

minhoryang commented Mar 15, 2017

juice500ml commented Mar 18, 2017

punkyoon commented Mar 18, 2017

minhoryang commented Mar 18, 2017

minhoryang commented Mar 19, 2017

punkyoon Mar 12, 2017 •

edited

Loading

juice500ml Mar 13, 2017 •

edited

Loading

minhoryang Mar 12, 2017 •

edited

Loading

juice500ml Mar 12, 2017 •

edited

Loading

minhoryang Mar 13, 2017 •

edited

Loading