public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Mira Limbeck <m.limbeck@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [RFC log-tracker] rfc3339: move timezone offset compatibility code to
Date: Tue, 13 Feb 2024 15:54:09 +0100	[thread overview]
Message-ID: <20240213155409.61527821@rosa.proxmox.com> (raw)
In-Reply-To: <20230718160101.1070267-1-m.limbeck@proxmox.com>

Thanks for the patch!

looks ok - but as talked off-list and also hinted in the api-patch reply I
sent - if possible we could maybe get rid of the issue that the
start+endtime provided in the GUI is interpreted differently than the
current option, which is yet again different to what I'd expect:
Testcase is '2023-09-21 13:52' as starttime '2023-09-21 15:00' as endtime
* with both patches applied the first mail shown is from 15:52 (offset +2h)
* with the currently released version the first mail is shown is from
  14:52 (offset +1h)
* I personally would expect no offset ;)
potentially we could not consider the client's timezone too much - and
define that the results are based on the server's timezone (or more
correctly on what's actually written in the logs)
If we want to consider the timezone of the client we should account for DST
i.e. if the client is currently (in February) in CET, that they would've
been in CEST in September - but I think that might be quite too much of a
hassle - and the break of displaying one time in the information line vs.
another in the actual logs sounds confusing

please adapt the tests for the next version

two tiny nits inline:

On Tue, 18 Jul 2023 18:01:00 +0200
Mira Limbeck <m.limbeck@proxmox.com> wrote:

> old time format parsing code
if possible - get this information also into the subject line (70
character limit)

> 
>..snip..
> @@ -2287,12 +2287,12 @@ fn parse_time_with_year(data: &'_ [u8], timezone_offset: time_t) -> Option<(time
>          std::str::from_utf8_unchecked(&timestamp_buffer[0..timestamp_len])
>      }) {
>          // TODO handle timezone offset in old code path instead
this patch fixes the TODO :) - get rid of it
> -        Ok(ltime) => Some((ltime + timezone_offset, data)),
> +        Ok(ltime) => Some((ltime, data)),
>          Err(_err) => None,
>      }
>  }
>  
> -fn parse_time_no_year(data: &'_ [u8], cur_year: i64, cur_month: i64) -> Option<(time_t, &'_ [u8])> {
> +fn parse_time_no_year(data: &'_ [u8], cur_year: i64, cur_month: i64, timezone_offset: time_t) -> Option<(time_t, &'_ [u8])> {
>      if data.len() < 15 {
>          return None;
>      }
> @@ -2397,7 +2397,7 @@ fn parse_time_no_year(data: &'_ [u8], cur_year: i64, cur_month: i64) -> Option<(
>          _ => &data[1..],
>      };
>  
> -    Some((ltime, data))
> +    Some((ltime - timezone_offset, data))
>  }
>  
>  type ByteSlice<'a> = &'a [u8];





      parent reply	other threads:[~2024-02-13 14:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-18 16:01 Mira Limbeck
2023-07-18 16:01 ` [pmg-devel] [RFC api] MailTracker: remove timezone offset Mira Limbeck
2024-02-13 14:04   ` Stoiko Ivanov
2023-08-04 12:30 ` [pmg-devel] [RFC log-tracker] rfc3339: move timezone offset compatibility code to Dominik Csapak
2024-02-13 14:54 ` Stoiko Ivanov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240213155409.61527821@rosa.proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=m.limbeck@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal