-
Notifications
You must be signed in to change notification settings - Fork 943
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
commit #945
base: master
Are you sure you want to change the base?
commit #945
Conversation
# Actor.objects.delete(1) | ||
# print(Actor.objects.all()) | ||
from models import Actor | ||
from managers import ActorManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to push changes to this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you revert the changes?
app/managers.py
Outdated
|
||
class ActorManager: | ||
def __init__(self) -> None: | ||
self._connection = sqlite3.connect("actors.db") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From requirements:
Create a database
cinema
where will be
def create_table(self) -> None: | ||
self._connection.execute( | ||
""" | ||
CREATE TABLE IF NOT EXISTS actors ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bad sql formatting. You can use this to make it pretty https://sqlformat.org/
Apply everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually here you can keep as is - sql formatter will spread lines too much. But check other queries - the least you can do is to put each keyword on new line - thats the standard
app/managers.py
Outdated
new_last_name: str | ||
) -> None: | ||
self._connection.execute( | ||
"UPDATE actors " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use triple quotes here as well
app/managers.py
Outdated
|
||
def create(self, first_name: str, last_name: str) -> None: | ||
self._connection.execute( | ||
f"""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
must be triple quotes here, double check this block again and make sure everything is aligned
# Actor.objects.delete(1) | ||
# print(Actor.objects.all()) | ||
from models import Actor | ||
from managers import ActorManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you revert the changes?
@@ -1,13 +0,0 @@ | |||
# from models import Actor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you didn't have to delete this file - just needed to revert changes that you did. Make it look like the file on master and that's it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert deletion of this file and i'll aprove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you didn't have to delete this file - just needed to revert changes that you did. Make it look like the file on master and that's it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert changes to main.py - it should look exactly like on master branch
app/managers.py
Outdated
def delete(self, id_delete: int) -> None: | ||
self._connection.execute( | ||
f""" | ||
"DELETE FROM {self.table_name} WHERE id = ?""", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you have this doublequotes here? Also all SQL keywords should start from new line:
DELETE FROM x
WHERE y
@@ -1,13 +0,0 @@ | |||
# from models import Actor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert deletion of this file and i'll aprove
app/actors.db
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove database file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert changes to main.py - it should look exactly like on master branch
cinema.sqlite
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove database file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to delete cinema.db
and restore main.py
to the way it is on master branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, approve but main.py
file should have not been changed at all - it needs to look exactly like on master
No description provided.