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

Client integration #78

Merged
merged 8 commits into from
Aug 9, 2021
Merged

Client integration #78

merged 8 commits into from
Aug 9, 2021

Conversation

gmacario
Copy link
Collaborator

No description provided.

@gmacario gmacario self-assigned this Jul 24, 2021
Signed-off-by: Gianpaolo Macario <[email protected]>
Signed-off-by: Gianpaolo Macario <[email protected]>
@gmacario
Copy link
Collaborator Author

@alv67 , @Raffone17: WDYT?
If you like it, please ACK and/or merge this PR.
I am driving to the seashore and have no time today 😄

@Raffone17 Raffone17 self-requested a review July 29, 2021 07:18
@Raffone17
Copy link
Collaborator

Merge this !

Copy link
Collaborator

@Raffone17 Raffone17 left a comment

Choose a reason for hiding this comment

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

💯

@gmacario
Copy link
Collaborator Author

gmacario commented Jul 30, 2021

Testing locally on gmpowerhorse (Ubuntu 20.04.2 LTS 64-bit)

git reset --hard
git describe --tags
docker-compose build --pull --no-cache
docker-compose up -d
docker-compose ps

Result:

gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ git describe --tags
v0.0.1-14-g9bc6caa
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ docker-compose build --pull --no-cache
Building server
Step 1/7 : FROM node:14.17.3-alpine
14.17.3-alpine: Pulling from library/node
Digest: sha256:fb6cb918cc72869bd625940f42a7d8ae035c4e786d08187b94e8b91c6a534dfd
Status: Image is up to date for node:14.17.3-alpine
 ---> f5f48375fc5d
Step 2/7 : WORKDIR /usr/src/app
 ---> Running in 764d60f6910a
Removing intermediate container 764d60f6910a
 ---> bd937a3ab18f
Step 3/7 : COPY ["package.json", "package-lock.json*", "npm-shrinkwrap.json*", "./"]
 ---> 71f862bac4d7
Step 4/7 : RUN npm install --silent
 ---> Running in d0807637a1a9
[bcrypt] Success: "/usr/src/app/node_modules/bcrypt/lib/binding/napi-v3/bcrypt_lib.node" is installed via remote
Love nodemon? You can now support the project via the open collective:
 > https://opencollective.com/nodemon/donate

[svelte-preprocess] Don't forget to install the preprocessors packages that will be used: node-sass/sass, stylus, less, postcss & postcss-load-config, coffeescript, pug, etc...
added 936 packages from 714 contributors and audited 624 packages in 40.846s

66 packages are looking for funding
  run `npm fund` for details

found 3 moderate severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details
...
> [email protected] build:server /usr/src/app
> rm -rf ./dist/ && tsc

Removing intermediate container ef635cec3d40
 ---> beb9af9aabe2
Step 7/7 : CMD ["npm", "run serve:prod"]
 ---> Running in cc7886e89349
Removing intermediate container cc7886e89349
 ---> 21eb09000438
Successfully built 21eb09000438
Successfully tagged blobfishes_server:latest
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ docker-compose up -d
Creating network "blobfishes_default" with the default driver
Creating blobfishes_server_1 ... done
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ docker-compose ps
       Name                      Command               State    Ports
---------------------------------------------------------------------
blobfishes_server_1   docker-entrypoint.sh npm r ...   Exit 1        
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$

TODO: Why did blobfishes_server_1 exit???

@@ -1,8 +1,8 @@
version: '3'

services:
static-pages:
build: client
# static-pages:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alv67 why did you remove service "static-pages"?
It was explicitly created to build the static website independently of the back-end.

I do not think it causes harm at all, on the other hand it simplifies things on CI/CD and for future integration of the https front-end - see #68

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right. I made some temporary modifications to do some tests. And pushed it to be able to transfer this WIP modification to my other working site. This was not intended to be pulled in this version. No problem at all to restore "static-pages" server.

"build:server": "rm -rf ./dist/ && tsc"
"build:server": "rm -rf ./dist/ && tsc",
"build:client": "cd client && npm run build",
"serve:prod": "npm run build:client && npm run serve:server",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To make code simpler and more readable I would recommend to sort entries alphabetically

@gmacario gmacario requested a review from alv67 July 30, 2021 05:37
@gmacario
Copy link
Collaborator Author

gmacario commented Jul 30, 2021

@alv67 please see my review comments on docker-compose.yml and package.json.

Also, it does not look like the command line to start container blobfishes_server_1 was correct:

gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ docker-compose up -d
Creating network "blobfishes_default" with the default driver
Creating blobfishes_server_1 ... done
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ docker-compose ps
       Name                      Command               State    Ports
---------------------------------------------------------------------
blobfishes_server_1   docker-entrypoint.sh npm r ...   Exit 1        
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$ docker-compose logs
Attaching to blobfishes_server_1
server_1  | 
server_1  | Usage: npm <command>
server_1  | 
server_1  | where <command> is one of:
server_1  |     access, adduser, audit, bin, bugs, c, cache, ci, cit,
server_1  |     clean-install, clean-install-test, completion, config,
server_1  |     create, ddp, dedupe, deprecate, dist-tag, docs, doctor,
server_1  |     edit, explore, fund, get, help, help-search, hook, i, init,
server_1  |     install, install-ci-test, install-test, it, link, list, ln,
server_1  |     login, logout, ls, org, outdated, owner, pack, ping, prefix,
server_1  |     profile, prune, publish, rb, rebuild, repo, restart, root,
server_1  |     run, run-script, s, se, search, set, shrinkwrap, star,
server_1  |     stars, start, stop, t, team, test, token, tst, un,
server_1  |     uninstall, unpublish, unstar, up, update, v, version, view,
server_1  |     whoami
server_1  | 
server_1  | npm <command> -h  quick help on <command>
server_1  | npm -l            display full usage info
server_1  | npm help <term>   search for help on <term>
server_1  | npm help npm      involved overview
server_1  | 
server_1  | Specify configs in the ini-formatted file:
server_1  |     /root/.npmrc
server_1  | or on the command line via: npm <command> --key value
server_1  | Config info can be viewed via: npm help config
server_1  | 
server_1  | [email protected] /usr/local/lib/node_modules/npm
server_1  | 
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$

Dockerfile Outdated Show resolved Hide resolved
Copy link
Collaborator

@alv67 alv67 left a comment

Choose a reason for hiding this comment

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

I made some cleaning to let this branch be merged so you can then marge also #45 .

NOTE: this is a WIP version, not to be considered as a Release.

Remains to do some works on the client :

  • Books Create/Edit form need to have a list of all authors to choose from and a list of already used genres.
  • View pages for single Author with his books
  • View pages for single Book with all insormation
  • Management for "owner", readers and reviews
  • more to come

Dockerfile Outdated Show resolved Hide resolved
Signed-off-by: Gianpaolo Macario <[email protected]>
@gmacario
Copy link
Collaborator Author

gmacario commented Aug 9, 2021

@alv67 as a matter of fact my suggestion was to replace

CMD ["npm", "run serve:prod"]

with

CMD ["npm", "run", "serve:prod"]

as on my local instance (Ubuntu 20.04.2 LTS 64-bit) the command you propose triggers a runtime error when the container is run

...
server_1        | 
server_1        | Usage: npm <command>
server_1        | 
server_1        | where <command> is one of:
server_1        |     access, adduser, audit, bin, bugs, c, cache, ci, cit,
server_1        |     clean-install, clean-install-test, completion, config,
server_1        |     create, ddp, dedupe, deprecate, dist-tag, docs, doctor,
server_1        |     edit, explore, fund, get, help, help-search, hook, i, init,
server_1        |     install, install-ci-test, install-test, it, link, list, ln,
server_1        |     login, logout, ls, org, outdated, owner, pack, ping, prefix,
server_1        |     profile, prune, publish, rb, rebuild, repo, restart, root,
server_1        |     run, run-script, s, se, search, set, shrinkwrap, star,
server_1        |     stars, start, stop, t, team, test, token, tst, un,
server_1        |     uninstall, unpublish, unstar, up, update, v, version, view,
server_1        |     whoami
server_1        | 
server_1        | npm <command> -h  quick help on <command>
server_1        | npm -l            display full usage info
server_1        | npm help <term>   search for help on <term>
server_1        | npm help npm      involved overview
server_1        | 
server_1        | Specify configs in the ini-formatted file:
server_1        |     /root/.npmrc
server_1        | or on the command line via: npm <command> --key value
server_1        | Config info can be viewed via: npm help config
server_1        | 
server_1        | [email protected] /usr/local/lib/node_modules/npm
server_1        | 
blobfishes_static-pages_1 exited with code 0
blobfishes_server_1 exited with code 1
gmacario@gmpowerhorse:~/github/aquariophilie/blobfishes (client-integration)$

I just added 5457989 to address this

@gmacario gmacario merged commit 733cfe9 into main Aug 9, 2021
@gmacario gmacario deleted the client-integration branch August 9, 2021 05:03
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.

3 participants