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

Editable dev container for connector proxy demo #2097

Merged
merged 11 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,19 @@ BACKEND_DEV_OVERLAY ?= spiffworkflow-backend/dev.docker-compose.yml
FRONTEND_CONTAINER ?= spiffworkflow-frontend
FRONTEND_DEV_OVERLAY ?= spiffworkflow-frontend/dev.docker-compose.yml

CONNECTOR_PROXY_CONTAINER ?= spiffworkflow-connector
CONNECTOR_PROXY_DEV_OVERLAY ?= connector-proxy-demo/dev.docker-compose.yml

YML_FILES := -f docker-compose.yml \
-f $(BACKEND_DEV_OVERLAY) \
-f $(FRONTEND_DEV_OVERLAY) \
-f $(CONNECTOR_PROXY_DEV_OVERLAY) \
-f $(ARENA_DEV_OVERLAY)

DOCKER_COMPOSE ?= RUN_AS=$(ME) docker compose $(YML_FILES)
IN_ARENA ?= $(DOCKER_COMPOSE) run --rm $(ARENA_CONTAINER)
IN_BACKEND ?= $(DOCKER_COMPOSE) run --rm $(BACKEND_CONTAINER)
IN_CONNECTOR_PROXY ?= $(DOCKER_COMPOSE) run --rm $(CONNECTOR_PROXY_CONTAINER)
IN_FRONTEND ?= $(DOCKER_COMPOSE) run --rm $(FRONTEND_CONTAINER)

SPIFFWORKFLOW_BACKEND_ENV ?= local_development
Expand All @@ -27,11 +37,6 @@ NODE_MODULES_DIR ?= spiffworkflow-frontend/node_modules
JUST ?=
ARGS ?=

YML_FILES := -f docker-compose.yml \
-f $(BACKEND_DEV_OVERLAY) \
-f $(FRONTEND_DEV_OVERLAY) \
-f $(ARENA_DEV_OVERLAY)

all: dev-env start-dev run-pyl
@true

Expand All @@ -43,7 +48,7 @@ build-images:
--build-arg GROUP_NAME=$(GROUP_NAME) \
$(JUST)

dev-env: stop-dev build-images poetry-i be-poetry-i be-db-clean fe-npm-i
dev-env: stop-dev build-images poetry-i cp-poetry-i be-poetry-i be-db-clean fe-npm-i
@true

start-dev: stop-dev
Expand Down Expand Up @@ -97,6 +102,18 @@ be-tests: be-clear-log-file
be-tests-par: be-clear-log-file
$(IN_BACKEND) poetry run pytest -n auto -x --random-order $(ARGS) tests/spiffworkflow_backend/$(JUST)

cp-sh:
$(IN_CONNECTOR_PROXY) /bin/bash

cp-poetry-i:
$(IN_CONNECTOR_PROXY) poetry install

cp-poetry-lock:
$(IN_CONNECTOR_PROXY) poetry lock --no-update

cp-logs:
docker logs -f $(CONNECTOR_PROXY_CONTAINER)

fe-lint-fix:
$(IN_FRONTEND) npm run lint:fix

Expand Down Expand Up @@ -155,6 +172,7 @@ include event-stream/demo.mk
start-dev stop-dev \
be-clear-log-file be-logs be-mypy be-poetry-i be-poetry-lock be-poetry-rm \
be-db-clean be-db-migrate be-sh be-sqlite be-tests be-tests-par \
cp-logs cp-poetry-i cp-poetry-lock \
fe-lint-fix fe-logs fe-npm-clean fe-npm-i fe-npm-rm fe-sh fe-unimported \
git-debranch git-debranch-offline \
poetry-i poetry-rm pre-commit ruff run-pyl \
Expand Down
2 changes: 1 addition & 1 deletion connector-proxy-demo/bin/run_server_locally
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ if [[ -z "${FLASK_DEBUG:-}" ]]; then
fi

export FLASK_SESSION_SECRET_KEY=super_secret_key
poetry run flask run -p 7004
poetry run flask run -p ${CONNECTOR_PROXY_PORT:-7004} --host=0.0.0.0
1 change: 1 addition & 0 deletions connector-proxy-demo/connector-example/.tool-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
python 3.11.0
504 changes: 504 additions & 0 deletions connector-proxy-demo/connector-example/LICENSE

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions connector-proxy-demo/connector-example/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# connector-example
Example Connector


## Happy Hacking

Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Clarify or remove the "Happy Hacking" section.

The "Happy Hacking" section title is present, but there's no content underneath it. This could be confusing for readers.

Consider one of the following options:

  1. Remove this section if it's not needed.
  2. Add relevant content that explains what developers can "hack" or modify in this connector.
  3. Rename the section to something more descriptive of its intended content, such as "Development Guidelines" or "Customization Tips".

When running the dev containers, you can edit this connector and see
changes reflected next time you run a process model with a Service Task
that calls this connector.
Comment on lines +1 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance overall README structure and content.

While the README provides basic information about the example connector and its development process, there are opportunities to improve its structure and content to make it more informative and user-friendly.

Consider restructuring the README as follows:

  1. Introduction: Expand on what the connector is and its purpose.
  2. Prerequisites: List any required software or setup steps.
  3. Getting Started: Provide step-by-step instructions for setting up the dev environment.
  4. Development Workflow: Explain how to make changes and test them.
  5. Usage: Show a basic example of how to use the connector in a process model.
  6. Troubleshooting: Add a section for common issues and their solutions.

This structure will provide a more comprehensive guide for developers working with the connector, aligning with the PR's objective of enhancing local development and training processes.

373 changes: 373 additions & 0 deletions connector-proxy-demo/connector-example/poetry.lock

Large diffs are not rendered by default.

67 changes: 67 additions & 0 deletions connector-proxy-demo/connector-example/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
[tool.poetry]
name = "connector-example"
version = "1.0.0"
description = "Example Connector for SpiffWorkflow Service Tasks"
authors = ["Jon Herron <[email protected]>"]
readme = "README.md"
packages = [{include = "connector_example", from = "src" }]

[tool.poetry.dependencies]
python = "^3.9"
spiffworkflow-connector-command = {git = "https://github.com/sartography/spiffworkflow-connector-command.git", rev = "main"}

[tool.poetry.group.dev.dependencies]
mypy = "^1.6.0"
ruff = "^0.0.292"
pytest = "^7.4.2"
Comment on lines +9 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Pin the spiffworkflow-connector-command dependency to a specific commit for better reproducibility

The spiffworkflow-connector-command dependency currently tracks the main branch, which may lead to reproducibility issues as the branch evolves. To ensure consistent builds, update the dependency to reference the specific commit 7e20603849ee609267ddc97c8433077e09ccd124:

[tool.poetry.dependencies]
python = "^3.9"
spiffworkflow-connector-command = {git = "https://github.com/sartography/spiffworkflow-connector-command.git", rev = "7e20603849ee609267ddc97c8433077e09ccd124"}

[tool.poetry.group.dev.dependencies]
mypy = "^1.6.0"
ruff = "^0.0.292"
pytest = "^7.4.2"
🔗 Analysis chain

Dependencies are well-defined, but consider pinning the Git dependency

The dependencies are appropriately specified, including essential development tools. However, the main dependency spiffworkflow-connector-command is fetched from a Git repository using the main branch. This approach might lead to reproducibility issues in the future.

Consider pinning this dependency to a specific commit or tag for better reproducibility. You can verify the latest release or commit using:

If there's a stable release, you might want to use that instead of the main branch.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the latest release or commit of spiffworkflow-connector-command

# Test: Get the latest release tag or commit hash
gh repo view sartography/spiffworkflow-connector-command --json 'latestRelease,defaultBranchRef'

Length of output: 154


Script:

#!/bin/bash
# Description: Retrieve the latest commit hash of the main branch for spiffworkflow-connector-command

gh api repos/sartography/spiffworkflow-connector-command/commits/main --jq '.sha'

Length of output: 122



[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

[tool.ruff]
select = [
"B", # flake8-bugbear
"C", # mccabe
"E", # pycodestyle error
# "ERA", # eradicate
"F", # pyflakes
"N", # pep8-naming
"PL", # pylint
"S", # flake8-bandit
"UP", # pyupgrade
"W", # pycodestyle warning
"I001" # isort
]

ignore = [
"C901", # "complexity" category
"PLR", # "refactoring" category has "too many lines in method" type stuff
"PLE1205" # saw this Too many arguments for `logging` format string give a false positive once
]

line-length = 130

# target python 3.10
target-version = "py310"

exclude = [
"migrations"
]

[tool.ruff.per-file-ignores]
"migrations/versions/*.py" = ["E501"]
"tests/**/*.py" = ["PLR2004", "S101"] # PLR2004 is about magic vars, S101 allows assert

[tool.ruff.isort]
force-single-line = true

[tool.mypy]
strict = true
disallow_any_generics = false
warn_unreachable = true
pretty = true
show_column_numbers = true
show_error_codes = true
show_error_context = true
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from typing import Any

from spiffworkflow_connector_command.command_interface import CommandErrorDict
from spiffworkflow_connector_command.command_interface import CommandResponseDict
from spiffworkflow_connector_command.command_interface import ConnectorCommand
from spiffworkflow_connector_command.command_interface import ConnectorProxyResponseDict


class Example(ConnectorCommand):
def __init__(self,
message: str,
):
self.message = message

def execute(self, _config: Any, _task_data: Any) -> ConnectorProxyResponseDict:
error: CommandErrorDict | None = None

return_response: CommandResponseDict = {
"body": {
"connector_response": f"You passed the example connector: '{self.message}'. Have a good day!",
},
"mimetype": "application/json",
}

result: ConnectorProxyResponseDict = {
"command_response": return_response,
"error": error,
"command_response_version": 2,
"spiff__logs": [],
}

return result
8 changes: 8 additions & 0 deletions connector-proxy-demo/dev.Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FROM python:3.11.6-slim-bookworm AS base

WORKDIR /app

RUN pip install --upgrade pip
RUN pip install poetry==1.8.1 pytest-xdist==3.5.0
Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider optimizing package installation and setting up editable installs

While upgrading pip and installing specific versions of poetry and pytest-xdist is good for reproducibility, consider the following improvements:

  1. Combine the RUN commands to reduce the number of layers in the image:
-RUN pip install --upgrade pip
-RUN pip install poetry==1.8.1 pytest-xdist==3.5.0
+RUN pip install --upgrade pip && \
+    pip install poetry==1.8.1 pytest-xdist==3.5.0
  1. To support editable installs for local development as mentioned in the PR objectives, consider adding steps to install the project in editable mode. This could be done by copying the project files and running poetry install --no-root in a subsequent step.

Would you like assistance in implementing these changes?

Committable suggestion was skipped due to low confidence.


CMD ["./bin/run_server_locally"]
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Startup script ./bin/run_server_locally is missing

The Dockerfile sets the default command to ./bin/run_server_locally, but this script does not exist in the repository. Please add the run_server_locally script to the ./bin directory or update the CMD instruction to reference the correct startup script.

🔗 Analysis chain

Verify the existence and permissions of the startup script

The use of CMD to set the default command is appropriate. However, ensure that the script ./bin/run_server_locally exists in your project and has the correct execute permissions.

To verify the script's existence and permissions, you can run the following commands in your local environment before building the image:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence and permissions of the startup script

# Check if the script exists
if [ -f "./bin/run_server_locally" ]; then
    echo "Script exists."
    # Check if the script is executable
    if [ -x "./bin/run_server_locally" ]; then
        echo "Script is executable."
    else
        echo "Script is not executable. Consider running: chmod +x ./bin/run_server_locally"
    fi
else
    echo "Script does not exist. Please ensure it's present in the ./bin directory."
fi

Length of output: 197

16 changes: 16 additions & 0 deletions connector-proxy-demo/dev.docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

services:
spiffworkflow-connector:
build:
context: connector-proxy-demo
dockerfile: dev.Dockerfile
user: "${RUN_AS}"
environment:
FLASK_DEBUG: "1"
POETRY_VIRTUALENVS_IN_PROJECT: "true"
XDG_CACHE_HOME: "/app/.cache"
env_file:
- path: .env
required: false
volumes:
- ./connector-proxy-demo:/app
20 changes: 18 additions & 2 deletions connector-proxy-demo/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions connector-proxy-demo/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ connector-aws = { git = "https://github.com/sartography/connector-aws.git"}
connector-http = {git = "https://github.com/sartography/connector-http.git"}
connector-slack = {git = "https://github.com/sartography/connector-slack.git"}
connector-smtp = {git = "https://github.com/sartography/connector-smtp.git"}
connector-example = {develop = true, path = "./connector-example" }

gunicorn = "^20.1.0"

[build-system]
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ services:
SPIFFWORKFLOW_BACKEND_URL: "http://localhost:${SPIFF_BACKEND_PORT:-8000}"

SPIFFWORKFLOW_BACKEND_BPMN_SPEC_ABSOLUTE_DIR: "/app/process_models"
SPIFFWORKFLOW_BACKEND_CONNECTOR_PROXY_URL: "http://spiffworkflow-connector:8004"
SPIFFWORKFLOW_BACKEND_CONNECTOR_PROXY_URL: "${SPIFFWORKFLOW_BACKEND_CONNECTOR_PROXY_URL:-http://spiffworkflow-connector:8004}"
SPIFFWORKFLOW_BACKEND_DATABASE_TYPE: "sqlite"
SPIFFWORKFLOW_BACKEND_DATABASE_URI: "sqlite:////app/db_volume/db.sqlite3"
SPIFFWORKFLOW_BACKEND_LOAD_FIXTURE_DATA: "false"
Expand Down
Loading