Skip to content

feat(server/machine-learning): Configurable port - #1386

Merged
alextran1502 merged 4 commits into
immich-app:mainfrom
samip5:feat/configureable-port
Jan 24, 2023
Merged

feat(server/machine-learning): Configurable port#1386
alextran1502 merged 4 commits into
immich-app:mainfrom
samip5:feat/configureable-port

Conversation

@samip5

@samip5 samip5 commented Jan 22, 2023

Copy link
Copy Markdown
Contributor

No description provided.

@vercel

vercel Bot commented Jan 22, 2023 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
immich-code-coverage ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 23, 2023 at 6:57PM (UTC)

@vercel

vercel Bot commented Jan 22, 2023

Copy link
Copy Markdown

@samip5 is attempting to deploy a commit to the immich Team on Vercel.

A member of the Team first needs to authorize it.

@bo0tzz

bo0tzz commented Jan 22, 2023

Copy link
Copy Markdown
Member

Since the applications are running in docker, why is this needed?

@vercel
vercel Bot temporarily deployed to Preview – immich-code-coverage January 22, 2023 10:17 Inactive
@samip5

samip5 commented Jan 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Since the applications are running in docker, why is this needed?

Because not everyone is able to use Docker (nor the default ports), and I know it might not be officially recommended, but it would help those use cases so I would see no harm in having these easily configurable.

@samip5
samip5 force-pushed the feat/configureable-port branch from 7c9632b to 90f147e Compare January 22, 2023 10:25
@vercel
vercel Bot temporarily deployed to Preview – immich-code-coverage January 22, 2023 10:26 Inactive
@samip5

samip5 commented Jan 23, 2023

Copy link
Copy Markdown
Contributor Author

The web container already is figuring out it's port from env, so I though it would make sense to do for others as well. Am I missing something here?

jrasm91
jrasm91 reviewed Jan 23, 2023
View reviewed changes
Comment thread machine-learning/src/main.ts Outdated
jrasm91
jrasm91 reviewed Jan 23, 2023
View reviewed changes
Comment thread server/apps/microservices/src/main.ts Outdated
@vercel
vercel Bot temporarily deployed to Preview – immich-code-coverage January 23, 2023 16:09 Inactive
jrasm91
jrasm91 reviewed Jan 23, 2023
View reviewed changes
Comment thread server/apps/immich/src/main.ts Outdated
@vercel
vercel Bot temporarily deployed to Preview – immich-code-coverage January 23, 2023 16:50 Inactive
jrasm91
jrasm91 approved these changes Jan 23, 2023
View reviewed changes

@jrasm91 jrasm91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. I don't think these will be necessary very often, but it doesn't hurt to make the port configurable via an env.

@vercel
vercel Bot temporarily deployed to Preview – immich-code-coverage January 23, 2023 18:57 Inactive
@alextran1502
alextran1502 merged commit 9d337bf into immich-app:main Jan 24, 2023
@samip5
samip5 deleted the feat/configureable-port branch January 24, 2023 06:48

This branch was previously deployed

1 inactive deployment
Preview – immich-code-coverage — 1e507956 Deployed Jan 23, 2023 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jrasm91 jrasm91 jrasm91 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@samip5 @bo0tzz @jrasm91 @alextran1502