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

Issues/8/create web application service #32

Merged
merged 8 commits into from
Oct 3, 2022

Conversation

leckijakub
Copy link
Contributor

@leckijakub leckijakub commented Sep 25, 2022

Debskij
Debskij previously approved these changes Sep 25, 2022
Copy link
Contributor

@Debskij Debskij left a comment

Choose a reason for hiding this comment

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

Hi @leckijakub, @MateuszKepczynski!
Thanks for your awesome contribution! Despite my comments, I would suggest creating a new issue to easily keep track of the most important changes that are currently marked as comments with the TODO tag.

POSTGRES_PASSWORD=splinter
POSTGRES_DB=splinter_db
POSTGRES_USER=postgres
POSTGRES_PASSWORD=1234
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to store passwords in a more secure manner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Debskij of course there are some ways but the question is if we want to introduce it now.
The simplest improvement would be to leave these variables empty and leave filling them to the person which would do the deployment. But that also means that each developer will have to do the same thing when they try to start the system from a freshly cloned repo.

I would suggest leaving credentials in configuration files until we plan the first release of the software.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great point, I feel that we should somehow keep the track of all the "shortcuts" that we currently use in order to simplify first local deployments. Maybe we can create a separate issue (or issues) for that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Debskij issue created #33

@@ -0,0 +1,33 @@
HELP.md
target/
!.mvn/wrapper/maven-wrapper.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not an expert in java, therefor it's just a question, should we include maven-wrapper.jar in git?
Found on stack and default gitignore by toptal for maven that it is usually placed in .gitignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question to @MateuszKepczynski

Copy link
Contributor

Choose a reason for hiding this comment

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

Its a stuff genereted from java code, tehres no need to keep this on github


spring.datasource.url=jdbc:postgresql://splinter_db:5432/splinter
spring.datasource.username=postgres
spring.datasource.password=1234
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before, is there any more secure way to store those passwords?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as in #32 (comment)

Comment on lines +11 to +16
VALUES ('MBI-2022', 'Metody badawcze w informatyce', 'pan.doktor@pg.edu.pl'),
('SOWW-2022', 'Systemy o wysokiej wydajności', 'pan.doktor@pg.edu.pl'),
('BS-2022', 'Bezpieczeństwo systemów', 'pan.doktor@pg.edu.pl'),
('BO-2022', 'Badania operacyjne', 'pan.doktor@pg.edu.pl'),
('BS-2022', 'Biblioteki cyfrowe', 'pan.doktor@pg.edu.pl'),
('UO-2022', 'Użyteczność oprogramowania', 'pan.doktor@pg.edu.pl');
Copy link
Contributor

Choose a reason for hiding this comment

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

Was it tested against multiple users? Can we already have subjects owned by two different users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question to @MateuszKepczynski

webapp/backend/src/main/resources/data.sql Outdated Show resolved Hide resolved
…ication.

JDK version used: openjdk_11
Maven version used: 3.6.3
…tend application.

In order to run the application in docker container, address 0.0.0.0
must be used to force PHP to listen to all available interfaces
instead of only internal `localhost` interface.
…stname.

After dockerizing database is not running on the same container as
backend and thus its address needs to be adjusted.

Additionally remove unfinished tests as it was preventing application
from building correctly.
@leckijakub
Copy link
Contributor Author

Hi @leckijakub, @MateuszKepczynski! Thanks for your awesome contribution! Despite my comments, I would suggest creating a new issue to easily keep track of the most important changes that are currently marked as comments with the TODO tag.

Hi @Debskij thanks!
@MateuszKepczynski I think this is an action for you as the owner of the java/PHP code. I think for now all this can be gathered in one issue and in the future we can create child issues for given functionalities.

@leckijakub
Copy link
Contributor Author

@Debskij issue for TODO comments created: #34

FYI @MateuszKepczynski

@leckijakub leckijakub merged commit af4ac2f into main Oct 3, 2022
@leckijakub leckijakub deleted the issues/8/create_web_application_service branch October 3, 2022 10:45
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.

Create web application service Check current status of Angular frontend application
4 participants