Skip to content

Feature/step1 - #2

Open
e-buerger wants to merge 34 commits into
mainfrom
feature/step1
Open

Feature/step1#2
e-buerger wants to merge 34 commits into
mainfrom
feature/step1

Conversation

@e-buerger

Copy link
Copy Markdown
Collaborator
  • setup database with sqlalchemy and sqlite

  • implement enums for workflow language, exec profile type and job status

  • implement method for creating jobs in job_factory

  • implement Execprofile class which only contains attributes and no methods

  • implement Job class which has abstract methods (prepare, exec, eval, finalize, cancel)

  • implement PythonJob class which inherits from Job and has the additional attribute inputs (this will be the class from which the class that is implemented by the user user inherits when he wants to use the python exec profile)

  • implement PyExecSession class which runs the method implemented by the user

  • implement tests for the functions in job_dao.py

@KerstenBreuerKerstenBreuer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Vielen Dank. Ein paar generelle Hinweise. Vielleicht können wir gleich drüber gehen.

Comment threadexec_manager/__init__.py Outdated
# limitations under the License.

"""Short description of package""" # Please adapt to package
"""backend""" # Please adapt to package

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""backend"""# Please adapt to package
"""
Apackagemanagingexecutionofjobsinawaythatisagnosticto
-theworkflowexecutionenvironment
-thelanguageusedtodescribetheworkflow
"""

Comment threadexec_manager/dao/__init__.py Outdated
Comment on lines +1 to +2
# Copyright 2021 - 2022 Universität Tübingen, DKFZ and EMBL
# for the German Human Genome-Phenome Archive (GHGA)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copyright muss noch angepasst werden

Comment threadexec_manager/wf_lang_type.py Outdated
"""enumerate workflow language types"""

CWL = "cwl"
"""cwl language"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doc strings werden normalerweise nur an den Anfang von einer Klasse, Funktion, oder einem Modul gestellt.

Comment threadexec_manager/job.py Outdated
Comment on lines +18 to +24
from uuid import UUID

from exec_manager.exec_profile import ExecProfile
from exec_manager.job_status_type import JobStatusType


class Job:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Job sollte am Besten eine abstrakte Klasse sein:

Suggested change
fromuuidimportUUID
fromexec_manager.exec_profileimportExecProfile
fromexec_manager.job_status_typeimportJobStatusType
classJob:
fromabcimportABC, abstractmethod
fromuuidimportUUID
fromexec_manager.exec_profileimportExecProfile
fromexec_manager.job_status_typeimportJobStatusType
classJob(ABC):

Comment threadexec_manager/job.py Outdated
Comment on lines +75 to +85
def prepare(self) -> None:
"""
Prepares the job.

Parameters
----------

Returns
-------
NONE
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Das hier sollten abstracte methoden sein:

Suggested change
defprepare(self) ->None:
"""
Preparesthejob.
Parameters
----------
Returns
-------
NONE
"""
@abstractmethod
defprepare(self) ->None:
"""
Preparesthejob.
Parameters
----------
Returns
-------
NONE
"""
...


from sqlalchemy import JSON, Column, String
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm.decl_api import DeclarativeMeta

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Du kannst den UUID Typ von SQLalchemy benutzen:

Suggested change
fromsqlalchemy.orm.decl_apiimportDeclarativeMeta
importuuid
fromsqlalchemy.orm.decl_apiimportDeclarativeMeta
fromsqlalchemy.dialects.postgresqlimportUUID

Comment threadexec_manager/dao/db_models.py Outdated
id = Column(Integer, primary_key=True)
name = Column(String, nullable=False)
active = Column(Boolean, nullable=False)
job_id = Column(String, primary_key=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Und hier dann:

Suggested change
job_id=Column(String, primary_key=True)
job_id=Column(UUID(as_uuid=True), default=uuid.uuid4, primary_key=True)

So wird die UUID automatisch vergeben.

Comment threadexec_manager/dao/job_dao.py Outdated
Comment on lines +62 to +63
job_id = generate_job_id()
job_id_str = str(job_id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Siehe die Vorschläge im db_model. Dort habe ich einen default generator für die id eingefügt. Dann musst du dich hier nicht mehr drum kümmern.

Comment threadexec_manager/exec_profile.py Outdated
from exec_manager.wf_lang_type import WfLangType


class ExecProfile:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Für das ExecProfile bietet es sich an entweder eine Dataclass oder ein pydantic model zu verwenden, da diese Klasse lediglich daten speichern soll.

Siehe z.B. hier: https://pydantic-docs.helpmanual.io/#example

Comment threadexec_manager/dao/job_dao.py Outdated
Comment on lines +65 to +70
exec_profile_json = json.dumps(
{
"exec_profile_type": exec_profile.exec_profile_type.value,
"wf_lang": exec_profile.wf_lang.value,
}
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wenn das ExecProfile auf einem pydantic model beruht kann man einfach schreiben:

Suggested change
exec_profile_json=json.dumps(
{
"exec_profile_type": exec_profile.exec_profile_type.value,
"wf_lang": exec_profile.wf_lang.value,
}
)
exec_profile_dict=exec_profile.to_dict()

Das hier returned ein dict. Also kein JSON. Aber die Conversion zu JSON kann man auch sqlalchemy machen lassen.

create classfiles
adapting code for passing precommit
adapt code for passing precommit
adapt code for passing precommit
implement a few functions
change type of db columns
satisfy pre-commit
implement exec method of PythonJob
suppres bandit warnigns and add comments
remove __init__.py from dao directory
remove context manager
fix session error in job_dao.py
change db commands
change db commands
change db commands
fix import errors by using absolute path and an __init__ file
remove content from exec method
add license headers
add license headers
implement PyExecSession
implement unit tests for job_dao
satisfy pipeline
satisfy pipeline
add license header
update README
update doc strings; change job_dao functions for better unit tests
change engine type
Sign up for freeto 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

@e-buerger@KerstenBreuer