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

Initial scaffolding #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

quarckster
Copy link
Contributor

  • added application skeleton
  • added taretto cli tool: taretto application create --path some_path

@mshriver
Copy link
Contributor

Just used this branch to create a new repo, its great! Thanks @quarckster, you rock

zip_safe = False
include_package_data = True
packages = find:
entry_points = file:entry_points.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
entry_points = file:entry_points.txt
entry_points = file:entry_points.txt
install_requires =
taretto
importscan

import importscan

from pkg_resources import iter_entry_points
from taretto.modeling import EntityCollections
Copy link
Contributor

Choose a reason for hiding this comment

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

This import is breaking with the use of apipkg.initpkg, as this modifies the namespace and results in only taretto.ui and taretto.navigate resolving when taretto is installed as a dependency.

I played with adding modeling to the exported namespace, but class imports from the module failed.

❯ git diff
diff --git a/src/taretto/__init__.py b/src/taretto/__init__.py
index b39d42a..4aa38c9 100644
--- a/src/taretto/__init__.py
+++ b/src/taretto/__init__.py
@@ -2,6 +2,7 @@ from apipkg import initpkg as __initpkg
 
 # fmt: off
 __initpkg(__name__, {
+    "modeling": "taretto.modeling",
     "navigate": "navmazing",
     "ui": {
         "core": "widgetastic.widget",  # todo: html widget library here, limit to base
❯ pip freeze |grep taretto
-e git+ssh://git@github.com/mshriver/taretto.git@3c102517377b4f994fbb3fe421de5ea3cbf6dd7f#egg=taretto
❯ ipython
Python 3.8.12 (default, Aug 30 2021, 00:00:00) 
Type 'copyright', 'credits' or 'license' for more information
IPython 8.0.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from taretto import modeling

In [2]: from taretto.modeling import EntityCollections
Fatal Python error: Cannot recover from stack overflow.
Python runtime state: initialized

Thread 0x00007fe5cba00640 (most recent call first):
  File "/usr/lib64/python3.8/threading.py", line 302 in wait
  File "/usr/lib64/python3.8/threading.py", line 558 in wait
  File "/home/mshriver/repos/satqe/robottelo/.robottelo/lib64/python3.8/site-packages/IPython/core/history.py", line 829 in run
  File "/home/mshriver/repos/satqe/robottelo/.robottelo/lib64/python3.8/site-packages/IPython/core/history.py", line 60 in only_when_enabled
  File "/home/mshriver/repos/satqe/robottelo/.robottelo/lib64/python3.8/site-packages/decorator.py", line 232 in fun
  File "/usr/lib64/python3.8/threading.py", line 932 in _bootstrap_inner
  File "/usr/lib64/python3.8/threading.py", line 890 in _bootstrap

Current thread 0x00007fe5dad43740 (most recent call first):
  File "/home/mshriver/repos/satqe/robottelo/.robottelo/lib64/python3.8/site-packages/apipkg/__init__.py", line 148 in importobj
  File "/home/mshriver/repos/satqe/robottelo/.robottelo/lib64/python3.8/site-packages/apipkg/__init__.py", line 290 in getmod
  File "/home/mshriver/repos/satqe/robottelo/.robottelo/lib64/python3.8/site-packages/apipkg/__init__.py", line 305 in __getattribute__
  <loop on above 3 lines>
  ...
[1]    324960 IOT instruction (core dumped)  ipython




@attr.s
class BaseEntity(object):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing the connection to sentaku.Element, even though taretto.context.Element, am I missing something or is that not part of this PR?

The cookie cutter application has implementation contexts, but no sentaku.Element classes that can have contextual methods/properties without inheriting both BaseEntity and taretto.context.Element?

Copy link
Collaborator

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

at first glance this is too close to iqe - mirroring some of its mistakes

additionally the skeleton should be a own evolve-able it repo so we can directly use cookiecutter instead of intensely tangleing the templates and theframework

@@ -0,0 +1,66 @@
"""Project path helpers
Copy link
Collaborator

Choose a reason for hiding this comment

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

based on what this file does, i would prefer we never add it

from collections import Callable

import attr
from cached_property import cached_property
Copy link
Collaborator

Choose a reason for hiding this comment

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

use the stdlib one

def filter(self, filter):
filters = self.filters.copy()
filters.update(filter)
return attr.evolve(self, filters=filters)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i suspect having a random dict as filters will set us up for pain

i'd like to see filters separated from collections to avoid the down payments that will eventually come from bringing them too close together



@attr.s
class CollectionProperty(object):
Copy link
Collaborator

Choose a reason for hiding this comment

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

starting to get the impression, this ought to land in sentaku, not taretto

[{{cookiecutter.application_name}}.application_collections]

[pytest11]
{{cookiecutter.application_name}} = {{cookiecutter.application_name}}.pytest_plugin
Copy link
Collaborator

Choose a reason for hiding this comment

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

pytest plugin all the things is not a good pattern per se, shouldn't be the default

return cls.for_entity(obj, *k, **kw).filter(filt)

def instantiate(self, *args, **kwargs):
return self.ENTITY.from_collection(self, *args, **kwargs)

Choose a reason for hiding this comment

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

it may also be nice to create an alias to instantiate with __call__ this would make it so people can "instantiate" from the collection

from {{cookiecutter.application_name}}.base.application.implementations.web_ui import ViaWebUI


class Application(object):

Choose a reason for hiding this comment

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

not specific to this pr, but is there a reason why we're hanging on to py2 support?

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.

None yet

4 participants