search

anatol / pacoloco

Caching proxy server for Arch Linux pacman
MIT License
199 stars 30 forks source link

log with timestamp #63

Closed HarHarLinks closed 1 year ago

HarHarLinks commented 1 year ago

it would be helpful to be able to track when things happened in the log, for example #64. currently the log lines don't include timestamps. i would suggest adding them.

Focshole commented 1 year ago

This should be done probably within the docker container only, on system logs it would clutter quite a lot the output (and they include the timestamp already).

HarHarLinks commented 1 year ago

indeed TIL one can use docker logs --timestamps, however that is UTC so can be confusing to read. suggestions? could there be an argument added to pacoloco to toggle logging timestamps according to the set TZ env var or similar?

Focshole commented 1 year ago

indeed TIL one can use docker logs --timestamps, however that is UTC so can be confusing to read. suggestions? could there be an argument added to pacoloco to toggle logging timestamps according to the set TZ env var or similar?

Added an option which enables log.SetFlags(log.LstdFlags) which (hopefully) solves this. Please, if you can, test that it actually adds timestamps to the output as expected by building it!

Focshole commented 1 year ago

Anyway, duplicate of #56

HarHarLinks commented 1 year ago

duplicate

that issue is a compound issue. might i suggest to track logging here and the remaining issue there?

building your fix-7 branch at ba10e96 looks good:

pacoloco  | pacoloco.go:103: Reading config file from /etc/pacoloco.yaml
pacoloco  | 2023/02/25 16:02:26 The prefetching routine will be run on 2023-02-26 03:00:00.000100725 +0000 UTC m=+39453.822110301
pacoloco  | 2023/02/25 16:02:26 Starting server at port 9129

how do I set my preferred time zone?

Focshole commented 1 year ago

how do I set my preferred time zone?

I'd go with changing it in the container, not sure if it works

HarHarLinks commented 1 year ago

your change combined with #66 resolves this in my eyes:

pacoloco  | pacoloco.go:103: Reading config file from /etc/pacoloco.yaml
pacoloco  | 2023/02/25 17:10:20 The prefetching routine will be run on 2023-02-26 03:00:00.000106 +0100 CET m=+35379.184080755
pacoloco  | 2023/02/25 17:10:20 Starting server at port 9129
Focshole commented 1 year ago

I don't know much about logging formats, maybe there are better/more standard flags. I admit it, I had simply googled how to add the timestamp on golang's log library and enabled it when a config value is set 😅

HarHarLinks commented 1 year ago

The combination of your change to the logger and my change to the dockerfile seems the right way to do it to me.

HarHarLinks commented 1 year ago

@Focshole something in your branch broke dbs for me: pacoloco delivers empty files for *.db.sig when there is no such file available on a mirror (Checking upstream availability of https://geo.mirror.pkgbuild.com/core/os/x86_64/core.db.sig ...). That leads to pacman breaking completely as it tries to verify that empty signature file which obviously can not be expected to work:

error: GPGME error: No data
error: failed to synchronize all databases (unexpected error)

You probably changed the behaviour unintentionally when refactoring in ef08f88. Could you please split the changes that address various issues from your branch into distinct branches so they can be merged cleanly (1 PR per logical change) and without introducing this error?

Focshole commented 1 year ago

Weird, did it create an empty file and sent it to the client? It is not an intended behavior

Could you please split the changes that address various issues from your branch into distinct branches so they can be merged cleanly (1 PR per logical change)

I would and I'd like to know how to do it, but it would take me hours to learn how to do so (I am terribly bad at using git, doing rebase and editing history, @anatol knows something about it) which I don't have right now, sorry. I tried fixing that issue here because it took me a few mins

HarHarLinks commented 1 year ago

I can not find empty .db.sig files in the pacoloco cache, only the client /var/lib/pacman/sync.

To split off commits in this situation you can use git cherry-pick like this:

  1. update your fork's master by clicking the button on https://github.com/Focshole/pacoloco image
  2. git checkout master && git pull update your local copy
  3. git checkout -b fix-64 to create a new branch dedicated to fix issue #64
  4. git cherry-pick ba10e96 which is the short commit id of https://github.com/anatol/pacoloco/commit/ba10e96edc6e7a8f7aa42b0610a2cc11452d04b3
  5. git push and open a PR.

In case of 2f20faf however you have done multiple things in a single commit. It is not good practice to do this for exactly this reason: if it isn't atomic, it is hard to revert or pick a single thing. You will have to manually copy the required changes to a new branch:

  1. git checkout master && git pull update your local copy
  2. git checkout -b fix-63 to create a new branch dedicated to fix issue #63
  3. edit the files manually based on 2f20faf as required
  4. git add the files and git commit
  5. git push and open a PR.
Focshole commented 1 year ago

Hopefully I didn't make a mess, TIL a bit about how to use cherry-pick. Thank you very much!