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

Create dockerfile #154

Merged
merged 16 commits into from
Feb 20, 2023
Merged

Create dockerfile #154

merged 16 commits into from
Feb 20, 2023

Conversation

haneslinger
Copy link
Collaborator

💚 docker start buildingmotif -i

root@235da402f95c:/# ls
bin  boot  buildingmotif  dev  etc  home  lib  lib64  media  mnt  opt  proc  root  run	sbin  srv  sys	tmp  usr  var

root@235da402f95c:/# cd buildingmotif/

root@235da402f95c:/buildingmotif# poetry run python
Skipping virtualenv creation, as specified in config file.
Python 3.8.15 (default, Oct 26 2022, 04:01:22) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from buildingmotif import BuildingMOTIF
>>> from buildingmotif.dataclasses import Model, Library
>>> 
>>> bm = BuildingMOTIF("sqlite://") # in-memory
>>> 
>>> lib = Library.load(ontology_graph="./libraries/brick/Brick-subset.ttl")   
>>> len(lib.get_templates())
824

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@haneslinger
Copy link
Collaborator Author

@TShapinsky Great comments, addressed.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@TShapinsky
Copy link
Member

@haneslinger I like where this is going. I'm not sure it makes sense to merge into develop right now since this seems to be for doing setup testing on rancher, right?

@haneslinger
Copy link
Collaborator Author

@TShapinsky, I mean, I don't see why we wouldn't push it, but I don't feel strongly about it.

@gtfierro
Copy link
Collaborator

Any reason why we wouldn't push?

@TShapinsky
Copy link
Member

Any reason why we wouldn't push?

This is a good question. I don't have strong feelings about this particular PR. If people feel it makes sense to merge we can merge.

My general apprehension comes from trying to keep repository fairly clean by not adding things which aren't integral to the library. Right now we don't have any workflows or documentation which is applicable to the Dockerfile and by itself the container doesn't do anything, you have to open a terminal into it and start things manually. I don't see this as a deficiency of the Dockerfile as this is exactly what @haneslinger and I had talked about making, but it just seems of limited utility to anyone else at this point.

From the other side, it is a useful Dockerfile to have and easier for each of us to use if we have it merged. 🤷

Copy link
Member

@MatthewSteen MatthewSteen left a comment

Choose a reason for hiding this comment

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

LGTM. No preference on merging or not. Looks like Podman can use Dockerfiles, so that's something I'll probably try in the future.

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2023

Codecov Report

Base: 73.42% // Head: 73.54% // Increases project coverage by +0.12% 🎉

Coverage data is based on head (88fe21b) compared to base (fc14b0f).
Patch coverage: 88.23% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #154      +/-   ##
===========================================
+ Coverage    73.42%   73.54%   +0.12%     
===========================================
  Files           31       31              
  Lines         2013     2026      +13     
===========================================
+ Hits          1478     1490      +12     
- Misses         535      536       +1     
Impacted Files Coverage Δ
buildingmotif/api/app.py 81.81% <0.00%> (ø)
buildingmotif/database/utils.py 88.46% <91.66%> (+2.74%) ⬆️
buildingmotif/database/tables.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@TShapinsky TShapinsky left a comment

Choose a reason for hiding this comment

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

Good to go once merge conflicts are resolved

docker-compose.yml Outdated Show resolved Hide resolved
Copy link
Member

@MatthewSteen MatthewSteen left a comment

Choose a reason for hiding this comment

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

Tried this with Podman and had one error. Haven't gotten it running yet but wanted to provide some intermediate feedback.

docker-compose.yml Outdated Show resolved Hide resolved
@haneslinger haneslinger merged commit 5dba327 into develop Feb 20, 2023
@haneslinger haneslinger deleted the Create-docker-file branch February 20, 2023 17:30
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

5 participants