-
Notifications
You must be signed in to change notification settings - Fork 27
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
Apps in an SQL DB #56
Conversation
I've implemented it but with two BUTS:
I am not sure whether to test it now or first solve "2". |
@apssouza22 I have a trouble finding Python code that loads I noticed the file Please, help to find this code or understand why it is missing. |
Now create app and list apps work. I propose you @apssouza22 to merge this PR to |
@vporton it is hard to test all these changes. Could you remove Alckemy from this MR? Make it simple. Addressing only one thing(store apps in the database) On the opensource world we need to make PRs very easy small and easy to follow because mantainers don't have time to dig into big changes |
It's not very easy to remove Alchemy from this set of changes because of subtle differences, but well I will do. |
I removed SQLAlchemy code. Please, accept this PR. It may contain errors, but in the present time the main method of software assurance is testing, not mathematical proof that it's correct. So, we will have enough time to test it, when it will be in the main branch, because our project is still unfinished. |
@vporton the app works fine today. We can not merge anything that breaks it. I have pulled your branch the app is not even starting. Please do some basic taste before open the PR.
Before that I had to fix the create table script
|
@apssouza22 Please, create branch Fixed table This error can be safely ignored. I tried to handle it with try:
self.add_user(User(name="Alex", password="123", email="[email protected]"))
self.dao.db.conn.commit()
except IntegrityError as e:
print("IGNORED EXCEPTION", e) # TODO: Remove this line.
self.dao.db.conn.rollback() |
@apssouza22 Oh, sorry, there really was a bug with |
Superseded by PR #62. |
Intended to fix issue #7.
This is a work in progress. I created this PR to track its dependency:
Depends on PR #55.
I just write
[WIP]
in the title instead of creating a draft pull request, because I had some awkward experience with draft pull request, something about having a hardship to convert it to a normal PR.