Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Creating DeleteCamera Task #271

Merged
merged 23 commits into from
Dec 24, 2024
Merged

Conversation

jue-henry
Copy link
Contributor

No description provided.

@jue-henry jue-henry self-assigned this Oct 10, 2024
@jue-henry jue-henry changed the title preliminary implementation Creating DeleteCamera Task Oct 10, 2024
@jue-henry jue-henry force-pushed the feature/229-add_delete_camera_task branch from 917a6e3 to 6a3b66d Compare November 21, 2024 00:28
@jue-henry jue-henry force-pushed the feature/229-add_delete_camera_task branch from 6a3b66d to 0c2a14a Compare December 4, 2024 23:36
src/api/db/models/Camera.ts Outdated Show resolved Hide resolved
@jue-henry jue-henry force-pushed the feature/229-add_delete_camera_task branch from a3ff1db to 8fabe38 Compare December 10, 2024 20:45
@jue-henry jue-henry marked this pull request as ready for review December 11, 2024 20:15
Copy link
Member

@nathanielrindlaub nathanielrindlaub left a comment

Choose a reason for hiding this comment

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

Hey @jue-henry looking good! Few questions/comments here but it's very close.

src/api/db/models/Project.ts Outdated Show resolved Hide resolved
src/api/db/models/Project.ts Show resolved Hide resolved
src/task/camera.ts Outdated Show resolved Hide resolved
src/task/camera.ts Show resolved Hide resolved
} else {
// make sure there's a Project.cameraConfig record for this camera
// in the default_project and create one if not
defaultProj = await Project.findOne({ _id: 'default_project' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nathanielrindlaub slightly different approach to this than the unregisterCamera method above, but I make sure default_project exists first and then create a CameraConfig in the default_project project before creating the project registration. This way we make sure the project exists first, instead of having a camera project registration point to a non-existant project, but also not sure if this is even an issue that needs to be addressed.

Copy link
Member

Choose a reason for hiding this comment

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

@jue-henry I don't think there not being a default_project in the DB should be an issue. It gets created when we initially seed the DB in the src/scripts/seedDB.js script. It is conceivable that someone might standing up their own Animl instance and not run the script, but that would cause a lot of other issues long before anyone every got to the point of deleting a camera, so I think it's safe to assume that default_project exists.

@@ -7,7 +7,7 @@ COPY ./ $HOME/
RUN apt-get update \
&& DEBIAN_FRONTEND=noninteractive apt-get install -y curl git

RUN export NODEV='20.12.2' \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nathanielrindlaub needed to upgrade for the docker tests to run. Merging for now but open to discussion

@jue-henry jue-henry merged commit db4309e into main Dec 24, 2024
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants