App review #4
App review #4
Conversation
Removed old projects, added new projects following the newest documentation: https://docs.carol.ai/docs/carol-app-dev
Online: removing not needed tags.
|
|
||
| WORKDIR /app | ||
| ADD requirements.txt /app/ | ||
| ADD batch_carolapp/ /app/batch_carolapp/ |
There was a problem hiding this comment.
RUN pip install -r requirements.txt should be before the ADD batch_carolapp/ /app/batch_carolapp/
then if you edit the app files, we dont need to rebuild all the image, just update this.
There was a problem hiding this comment.
Done. I adjusted the cookie-cutter projects that generated this code.
batch_carolapp/spec.json
Outdated
| @@ -5,13 +5,13 @@ | |||
| "memory": 4000, | |||
| "source": { | |||
| "type": "git", | |||
| "path": "https://${GHTOKEN}@github.com/totvslabs/carolapp-samples", | |||
| "path": "https://${GHTOKEN}@github.com/totvslabs/batch_carolapp", | |||
There was a problem hiding this comment.
this file is not needed.. This is to build directusing operator's APIS no?
There was a problem hiding this comment.
Ok, removing the file here and in cookie-cutter project.
| @@ -0,0 +1,44 @@ | |||
| # https://editorconfig.org/ | |||
There was a problem hiding this comment.
do we need this file?
maybe add to .gitignore?
There was a problem hiding this comment.
I think so, take a look:
EditorConfig helps maintain consistent coding styles for multiple developers working on the same project across various editors and IDEs. The EditorConfig project consists of a file format for defining coding styles and a collection of text editor plugins that enable editors to read the file format and adhere to defined styles. EditorConfig files are easily readable and they work nicely with version control systems.
It helps us to make the consistency cross editors.
Keeping the file.
| WORKDIR /app | ||
| ADD requirements.txt /app/ | ||
| ADD online_carolapp/ /app/online_carolapp/ | ||
|
|
There was a problem hiding this comment.
install before adding the files to use cache when rebuilding.
There was a problem hiding this comment.
Ok, I already replicated this change to batch and online. Here and in the cookie-cutter project.
|
Also, I dont agree to use this online app as our online app sample |
Rui's comments: RUN pip install -r requirements.txt should be before the ADD batch_carolapp/ /app/batch_carolapp/ then if you edit the app files, we dont need to rebuild all the image, just update this. source: totvslabs/carolapp-samples#4 (comment)
Rui's comments: RUN pip install -r requirements.txt should be before the ADD batch_carolapp/ /app/batch_carolapp/ then if you edit the app files, we dont need to rebuild all the image, just update this. source: totvslabs/carolapp-samples#4 (comment)
Rui's comments: RUN pip install -r requirements.txt should be before the ADD batch_carolapp/ /app/batch_carolapp/ then if you edit the app files, we dont need to rebuild all the image, just update this. source: #4 (comment)
File no more needed since we are building the docker image in Carol. Rui's suggestion: totvslabs/carolapp-samples#4 (comment)
I am assuming the same version available when I updated the repo.
@rafarui , I think this is the current Online App available. Right? Do you have a better suggestion? |
yes, a simple flask app.. this probably would work. and they have the full support of this from flask import Flask
app = Flask(__name__)
@app.route('/')
def hello_world():
return 'Hello, World!'maybe, better would be a flask app that implements authentication with carol |
Removing the dependency and keeping the pycarol dependency on python side.
Replacing projects by projects generated by cookie-cutter.