From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Daniel Tschlatscher <d.tschlatscher@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
pbs-devel@lists.proxmox.com, pmg-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH manager v2 4/7] revised task log API call for PVE
Date: Mon, 10 Oct 2022 15:10:15 +0200 [thread overview]
Message-ID: <018a731f-d9c3-b038-613d-f0d02df6355c@proxmox.com> (raw)
In-Reply-To: <09162367-74c0-a233-b913-a1b0e9477ef7@proxmox.com>
Am 10/10/2022 um 13:40 schrieb Daniel Tschlatscher:
> It does save about 20 lines of very redundant code in both pmg and pve
> each and should make it easy to implement potential other download
> calls. Though, that hinges on the question on how likely it is that
> there will be such a need.
20 lines is:
1) really not _that_ much
2) especially not dramatic as its just plain boilerplate info that won't
change
3) only your version needs that much ;-P Can be easily cut down of 16 lines
my $fh;
if ($compress) {
open($fh, '-|', "/usr/bin/gzip", "-c", "$file") or die "could not open file $file - $!";
} else {
open($fh, '<', $file) or die "could not open file $file- $!";
}
return {
download => {
fh => $fh,
stream => 1,
'content-encoding' => $compress ? 'gzip' : undef,
'content-type' => $content_type // 'text/plain',
'content-disposition' => 'attachment; filename="'. ($suggested_name // $file) .'"',
},
};
And independent of that, pve-common would be the wrong place for that helper, as it has no
control over how the http server takes the streaming hint, i.e., this is not a general
stream middleware but only prepare it in the specific format that our perl http server
expects it. So _iff_ it should go into pve-http-server, as otherwise any changes would need
and extra level of coordination on upgrade and possibly even make pve-common depend on
pve-http-server, introducing a circular dependency.
WARNING: multiple messages have this Message-ID
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Daniel Tschlatscher <d.tschlatscher@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
pbs-devel@lists.proxmox.com, pmg-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [pve-devel] [PATCH manager v2 4/7] revised task log API call for PVE
Date: Mon, 10 Oct 2022 15:10:15 +0200 [thread overview]
Message-ID: <018a731f-d9c3-b038-613d-f0d02df6355c@proxmox.com> (raw)
In-Reply-To: <09162367-74c0-a233-b913-a1b0e9477ef7@proxmox.com>
Am 10/10/2022 um 13:40 schrieb Daniel Tschlatscher:
> It does save about 20 lines of very redundant code in both pmg and pve
> each and should make it easy to implement potential other download
> calls. Though, that hinges on the question on how likely it is that
> there will be such a need.
20 lines is:
1) really not _that_ much
2) especially not dramatic as its just plain boilerplate info that won't
change
3) only your version needs that much ;-P Can be easily cut down of 16 lines
my $fh;
if ($compress) {
open($fh, '-|', "/usr/bin/gzip", "-c", "$file") or die "could not open file $file - $!";
} else {
open($fh, '<', $file) or die "could not open file $file- $!";
}
return {
download => {
fh => $fh,
stream => 1,
'content-encoding' => $compress ? 'gzip' : undef,
'content-type' => $content_type // 'text/plain',
'content-disposition' => 'attachment; filename="'. ($suggested_name // $file) .'"',
},
};
And independent of that, pve-common would be the wrong place for that helper, as it has no
control over how the http server takes the streaming hint, i.e., this is not a general
stream middleware but only prepare it in the specific format that our perl http server
expects it. So _iff_ it should go into pve-http-server, as otherwise any changes would need
and extra level of coordination on upgrade and possibly even make pve-common depend on
pve-http-server, introducing a circular dependency.
WARNING: multiple messages have this Message-ID
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Daniel Tschlatscher <d.tschlatscher@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
pbs-devel@lists.proxmox.com, pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [pve-devel] [PATCH manager v2 4/7] revised task log API call for PVE
Date: Mon, 10 Oct 2022 15:10:15 +0200 [thread overview]
Message-ID: <018a731f-d9c3-b038-613d-f0d02df6355c@proxmox.com> (raw)
In-Reply-To: <09162367-74c0-a233-b913-a1b0e9477ef7@proxmox.com>
Am 10/10/2022 um 13:40 schrieb Daniel Tschlatscher:
> It does save about 20 lines of very redundant code in both pmg and pve
> each and should make it easy to implement potential other download
> calls. Though, that hinges on the question on how likely it is that
> there will be such a need.
20 lines is:
1) really not _that_ much
2) especially not dramatic as its just plain boilerplate info that won't
change
3) only your version needs that much ;-P Can be easily cut down of 16 lines
my $fh;
if ($compress) {
open($fh, '-|', "/usr/bin/gzip", "-c", "$file") or die "could not open file $file - $!";
} else {
open($fh, '<', $file) or die "could not open file $file- $!";
}
return {
download => {
fh => $fh,
stream => 1,
'content-encoding' => $compress ? 'gzip' : undef,
'content-type' => $content_type // 'text/plain',
'content-disposition' => 'attachment; filename="'. ($suggested_name // $file) .'"',
},
};
And independent of that, pve-common would be the wrong place for that helper, as it has no
control over how the http server takes the streaming hint, i.e., this is not a general
stream middleware but only prepare it in the specific format that our perl http server
expects it. So _iff_ it should go into pve-http-server, as otherwise any changes would need
and extra level of coordination on upgrade and possibly even make pve-common depend on
pve-http-server, introducing a circular dependency.
next prev parent reply other threads:[~2022-10-10 13:10 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-07 8:56 [pve-devel] [PATCH http/common/manager/wt/backup/pmg v2] fix: #3971 Tasklog download button Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pve-devel] [PATCH proxmox-backup v2 1/7] make tasklog downloadable in the backup server backend Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pve-devel] [PATCH http-server v2 2/7] acknowledge content-disposition header Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-29 12:36 ` [pve-devel] applied: " Thomas Lamprecht
2022-09-29 12:36 ` [pmg-devel] applied: [pve-devel] " Thomas Lamprecht
2022-09-29 12:36 ` [pbs-devel] " Thomas Lamprecht
2022-09-07 8:56 ` [pve-devel] [PATCH common v2 3/7] stream file method for PMG and PVE Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pve-devel] [PATCH manager v2 4/7] revised task log API call for PVE Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-10-05 12:30 ` [pve-devel] " Thomas Lamprecht
2022-10-05 12:30 ` [pmg-devel] " Thomas Lamprecht
2022-10-05 12:30 ` [pbs-devel] " Thomas Lamprecht
2022-10-10 11:40 ` Daniel Tschlatscher
2022-10-10 11:40 ` [pmg-devel] " Daniel Tschlatscher
2022-10-10 11:40 ` [pbs-devel] " Daniel Tschlatscher
2022-10-10 13:10 ` Thomas Lamprecht [this message]
2022-10-10 13:10 ` [pmg-devel] " Thomas Lamprecht
2022-10-10 13:10 ` [pbs-devel] " Thomas Lamprecht
2022-09-07 8:56 ` [pve-devel] [PATCH pmg-api v2 5/7] revised task log download function in the PMG backend Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-08 9:36 ` [pve-devel] [pmg-devel] " Stefan Sterz
2022-09-08 9:36 ` Stefan Sterz
2022-09-08 9:36 ` [pbs-devel] " Stefan Sterz
2022-09-08 11:19 ` [pve-devel] " Daniel Tschlatscher
2022-09-08 11:19 ` Daniel Tschlatscher
2022-09-08 11:19 ` [pbs-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pve-devel] [PATCH widget-toolkit v2 6/7] Source file download call in central function Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pve-devel] [PATCH widget-toolkit v2 7/7] add task log download button in TaskViewer Daniel Tschlatscher
2022-09-07 8:56 ` [pmg-devel] " Daniel Tschlatscher
2022-09-07 8:56 ` [pbs-devel] " Daniel Tschlatscher
2022-09-08 9:40 ` [pve-devel] [pmg-devel] [PATCH http/common/manager/wt/backup/pmg v2] fix: #3971 Tasklog download button Stefan Sterz
2022-09-08 9:40 ` Stefan Sterz
2022-09-08 9:40 ` [pbs-devel] " Stefan Sterz
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=018a731f-d9c3-b038-613d-f0d02df6355c@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=d.tschlatscher@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=pmg-devel@lists.proxmox.com \
--cc=pve-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal