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

Add Webhook Utility #12

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

Conversation

poojasa7182
Copy link

No description provided.

@@ -27,12 +27,15 @@ services:
backend:
restart: always
container_name: pesticide_backend
command: bash -c "cd ./src && python check_db.py --service-name db --ip db --port 3306 && python manage.py collectstatic --noinput && python manage.py makemigrations pesticide_app && python manage.py makemigrations && python manage.py migrate && daphne -b 0.0.0.0 -p 8000 pesticide.asgi:application"
command: bash -c "cd ./src && python check_db.py --service-name db --ip db --port 3306 && python manage.py collectstatic --noinput && daphne -b 0.0.0.0 -p 8000 pesticide.asgi:application"
Copy link
Owner

Choose a reason for hiding this comment

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

Do not commit this modified docker-compose file. The modifications made were only for easy development, and cannot be used in a production setup.

- ./pesticide_backend:/backend
- type: bind
source: ./pesticide_backend
target: /backend
Copy link
Owner

Choose a reason for hiding this comment

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

Why was this necessary?

ip_hash;
server frontend:3000 fail_timeout=0;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Just like the docker-compose file, modifications made to this file were only meant for development setup, and are not meant to be committed.

.gitignore Outdated
@@ -76,6 +76,7 @@ pesticide_backend/src/backend-static

# config secrets
pesticide_backend/src/config/base.yml
.docker-compose.yml
Copy link
Owner

Choose a reason for hiding this comment

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

Do not git-ignore this file.

)

# from pesticide_app.models.webhookDetails import WebhookDetails
Copy link
Owner

Choose a reason for hiding this comment

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

Remove comments.

@@ -0,0 +1,20 @@
# Generated by Django 3.1.1 on 2022-01-30 18:50
Copy link
Owner

Choose a reason for hiding this comment

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

A better approach is to merge all these migration files into a single file. 5 migration files for a single model are too much IMO 😅

Merge these 5 files into a single migration file.

@@ -1,3 +1,4 @@
import imp
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this imported?


name = models.CharField(max_length=100, unique=True)
repository_name = models.CharField(max_length=500)
ssh_url = models.CharField(max_length=10000)
Copy link
Owner

Choose a reason for hiding this comment

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

If the max_length is this big, we might as well use Django's TextField.

BASE_DIR,
'config/base.yml'
))
BASE_CONFIGURATION = yaml.load(BASE_CONFIG_FILE, Loader=yaml.FullLoader)
Copy link
Owner

Choose a reason for hiding this comment

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

The config YAML file has already been loaded in settings.py file. Define the required variable in settings.py and then import the variable (like the flask token) from settings.py.

For example:

// in settings.py:
FLASK_TOKEN = BASE_CONFIGURATION["flaskToken"]["token"]

// in permissions.py:
from pesticide.settings import FLASK_TOKEN

@@ -0,0 +1,21 @@
from django.http import HttpResponse
Copy link
Owner

Choose a reason for hiding this comment

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

Remove unused imports.

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.

2 participants