-
Notifications
You must be signed in to change notification settings - Fork 0
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
Docker containers #65
base: main
Are you sure you want to change the base?
Conversation
Thanks @AnkurSheel! Probably best if someone with a bit of knowledge on Docker considers this pull request although I will try and read up over the coming days and take a look. How is your docker experience @jonhilt? See this discussion for background as well as Ankur's note above. |
I have some Docker experience (enough to have learned the hard way why .dockerignore can be very important!) I've had a go at running the script mentioned here and encountered an error: Microsoft ODBC Driver 17 for SQL Server : Login failed for user 'SA' If I get a moment I'll dig further in to see if it's something about my setup but is this anything you encountered @AnkurSheel? |
I just tried deleteing the images and running ./runDb.sh and it seems to be fine for me. Maybe this is the issue As an aside, I think we should eventually move away from IntegratedSecurity Mode |
Thanks @AnkurSheel. As it turns out I think it may have been operator error! I hadn't run ./runDb.sh first! Unless I missed something else, it seemed like I had to make a few small tweaks to get this to run (and be able to add time entries).
This made everything work here, but I'm conscious that Docker isn't exactly my forte so I may have just hacked this until it worked and you are very welcome to tell me if I missed something and didn't need to make these changes @AnkurSheel! |
Thats odd. ./runClient.sh also runs the db step from docker-compose.yml. ./runDb.sh just spins up the db so that you can still debug using your IDE. But glad its working for you now :) The changes seem correct. I didn't want to make too many changes to the code as part of this PR as I felt it required a broader conversation on what we want to do.
I think we can use https in docker but it might be a little complex. Maybe we could drive that through a redirect when hosting.
I didn't get as far as adding TimeEntries but an environment variable is what I would do as well with a value in appsettings as a sensible default for people who don't want to run the db from the container
Could be an environment variable as well but that's probably overkill. Makes it easy to manage the ports through docker-compose though |
ef7d8bc
to
360ed51
Compare
360ed51
to
b526ee6
Compare
@jonhilt Thanks for making the changes. |
Hi @AnkurSheel Thanks for your work on this. As mentioned on the roundup I've been investing in some Docker tutorials so hope to review the pull request this evening or tomorrow evening which may mean I have some questions for you. Other than that once @jonhilt is happy we can hopefully see this merged this week. Thanks again. |
Hi @AnkurSheel I'll apologise in advance for the newbie Docker questions however I was wondering if I could run a couple of things past you. After watching various tutorials I have a reasonable understanding on what each of the Docker files is doing and the purpose of the docker compose file so that's great. As I'm keen to keep this project "beginner friendly" I'm aiming to put together a high level guide outlining the steps required to utilise docker within the project. Firstly can I just check that my approach was correct?
After which I see the below within Docker. However when I select to browse Timekeeper_Client I get the below SSL error. I watched this YouTube tutorial on enabling HTTPS support for localhost however to get it working for now is there a step I'm missing that would allow it work via HTTP? A couple of other newbie questions.
Thanks in advance and @jonhilt may have other views on the above. |
Hey @DotNetDublin No worries. Always happy to answer any questions.
That works. However, you can compress the steps to
Can you try navigating to http://localhost:5002/? I see that the url has
That's the plan. But I would like to do it in a separate PR
I haven't worked with VS for a while now. But you should be able to add a solution folder
Probably not. It depends on how deep down the devops rabbit hole we want to go. I think its not a problem we need to solve unless we are at a massive scale.
Yes. I am not a big fan of persisting data for local db. I think we should seed the data on startup. This enables us to use the local db for integration tests. Not a strong opinion in this case and happy to go either way.
Sorry. I have been using Rider for C# dev and am not updated on docker support in Visual Studio. Unless, I am devving on the docker files, I prefer to use the command line to spin up/down the containers As an aside, you might find this article I wrote(on the back of this work) interesting How to run SQL Server in a Docker container |
Added a basic docker compose file to spin up the DB, client and API projects.
The 3 projects run on the following ports
If you runthe ./runClient.sh and navigate to http://localhost:5002, there are a few errors which I think we should fix soon . That means there is still a fair bit of work that needs to be done before everything runs smoothly but I think merging this in its current state has a few advantages