[PATCH ] Fix screenshots and debug reports paths.
From: Leonardo Garcia <lagarcia@br.ibm.com> The definitions of the screenshots and debug reports paths in Cherrypy config were pointing to a relative path that, when concatenated with the root path defined in the [/] section, were not mapping to the correct path when Kimchi is installed through its RPM package. This fix removes the use of the root path in the [/] config section and specifies the absolute path for all resources. That way we can easily handle running Kimchi directly from the git source tree or after a package installation --- in the later case the paths will not always be relative to the same root path, but will rather be spread in the file system according to the unix file system standard. Signed-off-by: Leonardo Garcia <lagarcia@br.ibm.com> --- src/kimchi/server.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/kimchi/server.py b/src/kimchi/server.py index ef8e701..6c7dfc1 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -65,8 +65,6 @@ class Server(object): CACHEEXPIRES = 31536000 configObj = { '/': {'tools.trailing_slash.on': False, - 'tools.staticdir.root': paths.prefix, - 'tools.staticfile.root': paths.prefix, 'request.methods_with_bodies': ('POST', 'PUT'), 'tools.nocache.on': True, 'tools.sessions.on': True, @@ -77,45 +75,45 @@ class Server(object): 'tools.kimchiauth.on': False}, '/css': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/css', + 'tools.staticdir.dir': '%s/ui/css' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False }, '/js': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/js', + 'tools.staticdir.dir': '%s/ui/js' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False }, '/libs': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/libs', + 'tools.staticdir.dir': '%s/ui/libs' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False, }, '/images': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/images', + 'tools.staticdir.dir': '%s/ui/images' % paths.prefix, 'tools.nocache.on': False }, '/data/screenshots': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'data/screenshots', + 'tools.staticdir.dir': config.get_screenshot_path(), 'tools.nocache.on': False }, '/data/debugreports': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'data/debugreports', + 'tools.staticdir.dir': config.get_debugreports_path(), 'tools.nocache.on': False, 'tools.kimchiauth.on': True, 'tools.staticdir.content_types': {'xz': 'application/x-xz'} }, '/config/ui/tabs.xml': { 'tools.staticfile.on': True, - 'tools.staticfile.filename': 'config/ui/tabs.xml', + 'tools.staticfile.filename': '%s/config/ui/tabs.xml' % paths.prefix, 'tools.nocache.on': True }, '/favicon.ico': { @@ -124,7 +122,7 @@ class Server(object): }, '/help': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/pages/help', + 'tools.staticdir.dir': '%s/ui/pages/help' % paths.prefix, 'tools.nocache.on': False } } -- 1.8.5.3
-- Tested-by: Paulo Vital <pvital@linux.vnet.ibm.com> Reviewed-by: Paulo Vital <pvital@linux.vnet.ibm.com> On Wed, 2014-02-26 at 12:38 -0300, Leonardo Garcia wrote:
From: Leonardo Garcia <lagarcia@br.ibm.com>
The definitions of the screenshots and debug reports paths in Cherrypy config were pointing to a relative path that, when concatenated with the root path defined in the [/] section, were not mapping to the correct path when Kimchi is installed through its RPM package.
This fix removes the use of the root path in the [/] config section and specifies the absolute path for all resources. That way we can easily handle running Kimchi directly from the git source tree or after a package installation --- in the later case the paths will not always be relative to the same root path, but will rather be spread in the file system according to the unix file system standard.
Signed-off-by: Leonardo Garcia <lagarcia@br.ibm.com> --- src/kimchi/server.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/kimchi/server.py b/src/kimchi/server.py index ef8e701..6c7dfc1 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -65,8 +65,6 @@ class Server(object): CACHEEXPIRES = 31536000 configObj = { '/': {'tools.trailing_slash.on': False, - 'tools.staticdir.root': paths.prefix, - 'tools.staticfile.root': paths.prefix, 'request.methods_with_bodies': ('POST', 'PUT'), 'tools.nocache.on': True, 'tools.sessions.on': True, @@ -77,45 +75,45 @@ class Server(object): 'tools.kimchiauth.on': False}, '/css': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/css', + 'tools.staticdir.dir': '%s/ui/css' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False }, '/js': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/js', + 'tools.staticdir.dir': '%s/ui/js' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False }, '/libs': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/libs', + 'tools.staticdir.dir': '%s/ui/libs' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False, }, '/images': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/images', + 'tools.staticdir.dir': '%s/ui/images' % paths.prefix, 'tools.nocache.on': False }, '/data/screenshots': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'data/screenshots', + 'tools.staticdir.dir': config.get_screenshot_path(), 'tools.nocache.on': False }, '/data/debugreports': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'data/debugreports', + 'tools.staticdir.dir': config.get_debugreports_path(), 'tools.nocache.on': False, 'tools.kimchiauth.on': True, 'tools.staticdir.content_types': {'xz': 'application/x-xz'} }, '/config/ui/tabs.xml': { 'tools.staticfile.on': True, - 'tools.staticfile.filename': 'config/ui/tabs.xml', + 'tools.staticfile.filename': '%s/config/ui/tabs.xml' % paths.prefix, 'tools.nocache.on': True }, '/favicon.ico': { @@ -124,7 +122,7 @@ class Server(object): }, '/help': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/pages/help', + 'tools.staticdir.dir': '%s/ui/pages/help' % paths.prefix, 'tools.nocache.on': False } }
Tested-by: Adriano Botega <abotega@linux.vnet.ibm.com> Reviewed-by: Adriano Botega <abotega@linux.vnet.ibm.com> Citando Leonardo Garcia <lagarcia@linux.vnet.ibm.com>:
From: Leonardo Garcia <lagarcia@br.ibm.com>
The definitions of the screenshots and debug reports paths in Cherrypy config were pointing to a relative path that, when concatenated with the root path defined in the [/] section, were not mapping to the correct path when Kimchi is installed through its RPM package.
This fix removes the use of the root path in the [/] config section and specifies the absolute path for all resources. That way we can easily handle running Kimchi directly from the git source tree or after a package installation --- in the later case the paths will not always be relative to the same root path, but will rather be spread in the file system according to the unix file system standard.
Signed-off-by: Leonardo Garcia <lagarcia@br.ibm.com> --- src/kimchi/server.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/kimchi/server.py b/src/kimchi/server.py index ef8e701..6c7dfc1 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -65,8 +65,6 @@ class Server(object): CACHEEXPIRES = 31536000 configObj = { '/': {'tools.trailing_slash.on': False, - 'tools.staticdir.root': paths.prefix, - 'tools.staticfile.root': paths.prefix, 'request.methods_with_bodies': ('POST', 'PUT'), 'tools.nocache.on': True, 'tools.sessions.on': True, @@ -77,45 +75,45 @@ class Server(object): 'tools.kimchiauth.on': False}, '/css': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/css', + 'tools.staticdir.dir': '%s/ui/css' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False }, '/js': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/js', + 'tools.staticdir.dir': '%s/ui/js' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False }, '/libs': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/libs', + 'tools.staticdir.dir': '%s/ui/libs' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, 'tools.nocache.on': False, }, '/images': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/images', + 'tools.staticdir.dir': '%s/ui/images' % paths.prefix, 'tools.nocache.on': False }, '/data/screenshots': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'data/screenshots', + 'tools.staticdir.dir': config.get_screenshot_path(), 'tools.nocache.on': False }, '/data/debugreports': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'data/debugreports', + 'tools.staticdir.dir': config.get_debugreports_path(), 'tools.nocache.on': False, 'tools.kimchiauth.on': True, 'tools.staticdir.content_types': {'xz': 'application/x-xz'} }, '/config/ui/tabs.xml': { 'tools.staticfile.on': True, - 'tools.staticfile.filename': 'config/ui/tabs.xml', + 'tools.staticfile.filename': '%s/config/ui/tabs.xml' % paths.prefix, 'tools.nocache.on': True }, '/favicon.ico': { @@ -124,7 +122,7 @@ class Server(object): }, '/help': { 'tools.staticdir.on': True, - 'tools.staticdir.dir': 'ui/pages/help', + 'tools.staticdir.dir': '%s/ui/pages/help' % paths.prefix, 'tools.nocache.on': False } } -- 1.8.5.3
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Aline, I am unhappy about that this patch was merged so quickly. 1) This patch was initially post on 2/26 23:38 and got merged 2:06AM 2/27, so people in other timezone didn't get any opportunity to review it. 2) I post a patch to fix the same problem on 2/26 17:03 that was earlier than Leonardo's patch. Why did the later patch be merged so quickly and my patch was left? As to Leonardo's patch, I don't think we should make every path absolute and we should use relative path as possible as we can , because we should not expose host file system to the front user as possible as we can. 2014/2/27 2:06, Aline Manera:
Applied. Thanks.
Regards,
Aline Manera
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Ming, Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed. Best regards, -- Paulo Ricardo Paz Vital <pvital@linux.vnet.ibm.com> IBM Linux Technology Center On Thu, 2014-02-27 at 10:32 +0800, Shu Ming wrote:
Aline,
I am unhappy about that this patch was merged so quickly. 1) This patch was initially post on 2/26 23:38 and got merged 2:06AM 2/27, so people in other timezone didn't get any opportunity to review it. 2) I post a patch to fix the same problem on 2/26 17:03 that was earlier than Leonardo's patch. Why did the later patch be merged so quickly and my patch was left? As to Leonardo's patch, I don't think we should make every path absolute and we should use relative path as possible as we can , because we should not expose host file system to the front user as possible as we can.
2014/2/27 2:06, Aline Manera:
Applied. Thanks.
Regards,
Aline Manera
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. I was planing to send screenshot patch later after all of us agree with
2014/2/27 21:47, Paulo Ricardo Paz Vital: the method to fix DebugReports, anyway this is another issue not directly linked to my point. My point was that: 1) when one people already worked on something, the later one should communicate with the former one to avoid duplicate effort 2) The patch should have some soaking time to be merged, say 24 hours, and people in other timezone can get an opportunity to review it.
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed.
In most cases ,we should use relative path to hide the host file systems from the front users. Only if it must, the absolute path can be used. I don't like the idea to change all the relative static path to absolute ones. IMO, we can only use absolute path for the debugreport and screenshot path. Even better, we can take more effort to re-organize the path to have a reasonable root for all of the path including debugreport and screenshot paths.
Best regards,
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. I was planing to send screenshot patch later after all of us agree with the method to fix DebugReports, anyway this is another issue not
2014/2/27 21:47, Paulo Ricardo Paz Vital: directly linked to my point. My point was that: 1) when one people already worked on something, the later one should communicate with the former one to avoid duplicate effort Sorry, Ming, I didn't see your patch in the mailing list before I sent mine. I apologize for that. 2) The patch should have some soaking time to be merged, say 24 hours, and people in other timezone can get an opportunity to review it. To be honest, I think this is not necessary. If we are talking about a new feature or a complex bug fix, I agree with you. But this is a fairly simple patch that directly solves a regression that was exposed by aother modification in late January. I have already discussed this in a scrum meeting: I think that simple patches like this one should not necessarily have a complete review process. Having to have a formal review process for every small patch is just a way to overload the contributors and make the project slower. But, anyway, I think this is a good topic for discussion on the next scrum meeting.
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed.
In most cases ,we should use relative path to hide the host file systems from the front users. The patch is not exposing anything to the front users. This path mapping is used internally by Cherrypy and never exposed in the UI. So I didn't get your point here. If someone really wants to know the real paths, well, this is an open source project: they will be able to see the
On 02/27/2014 12:39 PM, Shu Ming wrote: source and figure out the paths in the server anyway, regardless how they are typed in the source code. When Kimchi is installed through RPM, we are distributing files in various directories, just following what is described in the unix file system standard (to the best of my knowledge). Hence, using absolute paths is mandatory as there is no common path root shared between all files used by Kimchi. If you don't think this is the case, feel free to send a patch improving the patch I sent before. Best regards, Leonardo Garcia
Only if it must, the absolute path can be used. I don't like the idea to change all the relative static path to absolute ones. IMO, we can only use absolute path for the debugreport and screenshot path. Even better, we can take more effort to re-organize the path to have a reasonable root for all of the path including debugreport and screenshot paths.
Best regards,
2014/2/28 10:32, Leonardo Augusto Guimarães Garcia:
On 02/27/2014 12:39 PM, Shu Ming wrote:
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. I was planing to send screenshot patch later after all of us agree with the method to fix DebugReports, anyway this is another issue not
2014/2/27 21:47, Paulo Ricardo Paz Vital: directly linked to my point. My point was that: 1) when one people already worked on something, the later one should communicate with the former one to avoid duplicate effort Sorry, Ming, I didn't see your patch in the mailing list before I sent mine. I apologize for that. That is all right and it happens in open source community occasionally. My point is how we can improve the process to lessen duplicate efforts across different timezones.
2) The patch should have some soaking time to be merged, say 24 hours, and people in other timezone can get an opportunity to review it. To be honest, I think this is not necessary. If we are talking about a new feature or a complex bug fix, I agree with you. But this is a fairly simple patch that directly solves a regression that was exposed by aother modification in late January. I have already discussed this in a scrum meeting: I think that simple patches like this one should not necessarily have a complete review process. Having to have a formal review process for every small patch is just a way to overload the contributors and make the project slower. But, anyway, I think this is a good topic for discussion on the next scrum meeting.
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed. In most cases ,we should use relative path to hide the host file systems from the front users. The patch is not exposing anything to the front users. This path mapping is used internally by Cherrypy and never exposed in the UI. So I didn't get your point here. If someone really wants to know the real paths, well, this is an open source project: they will be able to see the source and figure out the paths in the server anyway, regardless how they are typed in the source code.
When Kimchi is installed through RPM, we are distributing files in various directories, just following what is described in the unix file system standard (to the best of my knowledge). Hence, using absolute paths is mandatory as there is no common path root shared between all files used by Kimch A common root path is possible, but it needs much effort to re-organize
The point goes to how we define what are simple patches. IMO, this patch we are talking about is not simple and obvious, though it only get a few lines. Because this patch change the layout of the static fies spreading and it get opportunity to break many URL links, CSS files, screen shot files &etc. the paths. I believe the " installed root path" or "src root path" may be the choice. But it seems that we have two many hard-coded paths in Kimchi, A non-trival effort is needed to achieve it.
If you don't think this is the case, feel free to send a patch improving the patch I sent before.
Best regards,
Leonardo Garcia
Only if it must, the absolute path can be used. I don't like the idea to change all the relative static path to absolute ones. IMO, we can only use absolute path for the debugreport and screenshot path. Even better, we can take more effort to re-organize the path to have a reasonable root for all of the path including debugreport and screenshot paths.
Best regards,
On 02/28/2014 10:32 AM, Leonardo Augusto Guimarães Garcia wrote:
On 02/27/2014 12:39 PM, Shu Ming wrote:
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. I was planing to send screenshot patch later after all of us agree with the method to fix DebugReports, anyway this is another issue not
2014/2/27 21:47, Paulo Ricardo Paz Vital: directly linked to my point. My point was that: 1) when one people already worked on something, the later one should communicate with the former one to avoid duplicate effort Sorry, Ming, I didn't see your patch in the mailing list before I sent mine. I apologize for that. 2) The patch should have some soaking time to be merged, say 24 hours, and people in other timezone can get an opportunity to review it. To be honest, I think this is not necessary. If we are talking about a new feature or a complex bug fix, I agree with you. But this is a fairly simple patch that directly solves a regression that was exposed by aother modification in late January. I have already discussed this in a scrum meeting: I think that simple patches like this one should not necessarily have a complete review process. Having to have a formal review process for every small patch is just a way to overload the contributors and make the project slower. But, anyway, I think this is a good topic for discussion on the next scrum meeting. yes. Careful is good to code quality. Most open source, the maintainer is higher skill than others, but he still careful for every patch. And he respects every developer's comments.
Actually, the vdsm we have joined is good example. For we do not want a patch followed a bug fix, right?
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed. In most cases ,we should use relative path to hide the host file systems from the front users. The patch is not exposing anything to the front users. This path mapping is used internally by Cherrypy and never exposed in the UI. So I didn't get your point here. If someone really wants to know the real paths, well, this is an open source project: they will be able to see the source and figure out the paths in the server anyway, regardless how they are typed in the source code.
When Kimchi is installed through RPM, we are distributing files in various directories, just following what is described in the unix file system standard (to the best of my knowledge). Hence, using absolute paths is mandatory as there is no common path root shared between all files used by Kimchi.
If you don't think this is the case, feel free to send a patch improving the patch I sent before.
Best regards,
Leonardo Garcia
Only if it must, the absolute path can be used. I don't like the idea to change all the relative static path to absolute ones. IMO, we can only use absolute path for the debugreport and screenshot path. Even better, we can take more effort to re-organize the path to have a reasonable root for all of the path including debugreport and screenshot paths.
Best regards,
Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
on 2014/02/28 10:32, Leonardo Augusto Guimarães Garcia wrote:
On 02/27/2014 12:39 PM, Shu Ming wrote:
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. I was planing to send screenshot patch later after all of us agree with the method to fix DebugReports, anyway this is another issue not
2014/2/27 21:47, Paulo Ricardo Paz Vital: directly linked to my point. My point was that: 1) when one people already worked on something, the later one should communicate with the former one to avoid duplicate effort Sorry, Ming, I didn't see your patch in the mailing list before I sent mine. I apologize for that. 2) The patch should have some soaking time to be merged, say 24 hours, and people in other timezone can get an opportunity to review it. To be honest, I think this is not necessary. If we are talking about a new feature or a complex bug fix, I agree with you. But this is a fairly simple patch that directly solves a regression that was exposed by aother modification in late January. I have already discussed this in a scrum meeting: I think that simple patches like this one should not necessarily have a complete review process. Having to have a formal review process for every small patch is just a way to overload the contributors and make the project slower. But, anyway, I think this is a good topic for discussion on the next scrum meeting.
Everyone has his/her own opinion on the necessary and sufficient condition of a quick bug. Sometimes quick fix leads to nasty bug. Sometimes maybe it's just a keyword typo, a quick fix is enough. IMHO, regardless how we define a quick fix, 24 hours delay should be quick enough for it. Unless it's an urgent && quick bug fix, we should not bypass the normal review process. Since Kimchi is an open source project, I doubt if there would be any "urgent" bugs that should bypass the 24 hour delay. There might be some bugs preventing us releasing a new version, but in this case I think we should delay the release rather than submitting quick fix and introducing new bugs. I don't think a 24 hours delay would much slow down Kimchi development speed. Sometimes it's the bug introduced from the quick patch that eats our time to fix it. We are building a new community, it's a wisdom to balance the development speed and code quality. I believe that every developer for each time he submits a patch/comment to a big community such as kernel and libvirt, he would be very careful, because it's about the reputation. In Kimchi we should build the same atmosphere, so that it leads the Kimchi project to a "smooth highway".
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed.
In most cases ,we should use relative path to hide the host file systems from the front users. The patch is not exposing anything to the front users. This path mapping is used internally by Cherrypy and never exposed in the UI. So I didn't get your point here. If someone really wants to know the real paths, well, this is an open source project: they will be able to see the source and figure out the paths in the server anyway, regardless how they are typed in the source code.
When Kimchi is installed through RPM, we are distributing files in various directories, just following what is described in the unix file system standard (to the best of my knowledge). Hence, using absolute paths is mandatory as there is no common path root shared between all files used by Kimchi.
If you don't think this is the case, feel free to send a patch improving the patch I sent before.
Best regards,
Leonardo Garcia
Only if it must, the absolute path can be used. I don't like the idea to change all the relative static path to absolute ones. IMO, we can only use absolute path for the debugreport and screenshot path. Even better, we can take more effort to re-organize the path to have a reasonable root for all of the path including debugreport and screenshot paths.
Best regards,
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. I was planing to send screenshot patch later after all of us agree with the method to fix DebugReports, anyway this is another issue not
2014/2/27 21:47, Paulo Ricardo Paz Vital: directly linked to my point. Yes, Usually the author make more investigations than the reviewer. But sometimes he may not tell more the details, such as what investigation he make, and what he will do the next step. if reviewer want to better, he should also make more investigation, but
On 02/27/2014 11:39 PM, Shu Ming wrote: this may take a reviewer long time. reviewer discusses with author is a good way.
My point was that: 1) when one people already worked on something, the later one should communicate with the former one to avoid duplicate effort 2) The patch should have some soaking time to be merged, say 24 hours, and people in other timezone can get an opportunity to review it.
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed.
In most cases ,we should use relative path to hide the host file systems from the front users. Only if it must, the absolute path can be used. I don't like the idea to change all the relative static path to absolute ones. IMO, we can only use absolute path for the debugreport and screenshot path. Even better, we can take more effort to re-organize the path to have a reasonable root for all of the path including debugreport and screenshot paths.
Best regards,
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
Ming,
Your patch solves only the DebugReports problem, while Leonardo's patch solves any difference between what UI expects and backend sets. Hi pvital, Any problem, you can give comments on the patch. such as if you think
On 02/27/2014 09:47 PM, Paulo Ricardo Paz Vital wrote: the patch can solves more problem, you can. but usually one patch just solve on problem, but not absolutely. And the reviewer can discuss with author more to get agreement at last. You can seem many patch in most open source projects are improved many version for many times discussion.
In addition, IMO the point you mentioned about not expose host file system to the front is the root cause of this bug. Many UI paths were broken because the paths set up by backend was not used or followed.
Best regards,
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
participants (8)
-
abotega＠linux.vnet.ibm.com -
Aline Manera -
Leonardo Augusto Guimarães Garcia -
Leonardo Garcia
-
Paulo Ricardo Paz Vital -
Sheldon -
Shu Ming -
Zhou Zheng Sheng