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

Write an evaluation of this approach #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roschaefer
Copy link

Have a look at how I understand how the microservice works and the comparison
at the bottom of the file. Please correct me if I get it wrong and feel free to
answer my open questions. I think the big advantage of this microservice
is that we don't have to settle for a fixed size of image resolutions
but have some sort of microservice which is doing the conversion on the
fly. However I believe this microservice adds a lot of complexity to our
code base. Currently I don't see the justification for it if we can more or
less achieve the same behaviour without any self-written service.

@roschaefer roschaefer force-pushed the evaluation branch 3 times, most recently from 71cf14b to aac1ea7 Compare January 9, 2020 18:47
Have a look at how I understand how the microservice works and the comparison
at the bottom of the file. Please correct me if I get it wrong and feel free to
answer my open questions. I think the big advantage of this microservice
is that we don't have to settle for a fixed size of image resolutions
but have some sort of microservice which is doing the conversion on the
fly. However I believe this microservice adds a lot of complexity to our
code base. Currently I don't see the justification for it if we can more or
less achieve the same behaviour *without any self-written service*.
I get:
```
You need to set up the project dependencies using the following commands:
curl -sS https://getcomposer.org/installer | php
Copy link
Owner

Choose a reason for hiding this comment

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

you need just instal composer

Copy link
Author

Choose a reason for hiding this comment

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

I followed those steps and got the error message mentioned in this PR.

```

I can access [localhost:8000](http://localhost:8000/) but the health check on [localhost:8000/health-check](http://localhost:8000/health-check) seems broken, I get HTTP 500 error and the message: "This page isn’t working" in Chromium.

Copy link
Owner

Choose a reason for hiding this comment

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

because composer not installed

[ws.human-connection.social/api/docs](https://ws.human-connection.social/api/docs)?
I would like to upload a file through the swagger web interface.
* Are requests to `/cdn/thumbnail/by-url` cached in some way? Like, are the
thumbnails saved on disk?
Copy link
Owner

Choose a reason for hiding this comment

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

this is only alpha version to the demonstration
all config can put in to .env file with basis .env.example
mysql and phpmyadmin only for local and demo installation - in live we not need him

| we add extra services on kubernetes (mysql, phpmyadmin) | no self-hosted services |
| ImageMagick for image resizing | sharp uses libvips - [they claim it's faster](https://github.com/lovell/sharp#sharp) |
| we have to configure a CDN on our own | Digital Ocean's CDN out of the box|

Copy link
Owner

Choose a reason for hiding this comment

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

I am not ready to provide a token for saving any files to my private server
cache and any data storage on my server for this reason are disabled in the demo

it all look notfriendly and like a desire to make any decision on PHP or from other developers !!!! :)
it is a microservice and the network can only use it through API and no matter what language it works in

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.

2 participants