You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/02/10 06:46:19 UTC
[kudu-CR] util: fix logged Kudu URL
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9280
Change subject: util: fix logged Kudu URL
......................................................................
util: fix logged Kudu URL
Previously we would log urls like:
http://kudu.apache.org/releases/1.3.0-SNAPSHOT/docs/...
Instead of:
http://kudu.apache.org/releases/1.3.0/docs/...
We should parse a number out of the supplied version string. This patch
does this in the gen_version_info script and adds a new auto-generated
macro KUDU_VERSION_NUMBER.
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M build-support/gen_version_info.py
M src/kudu/fs/fs_manager.cc
M src/kudu/util/version_info.cc
M src/kudu/util/version_info.h
M src/kudu/util/website_util.cc
5 files changed, 24 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/1
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
Patch Set 3:
I updated this to get rid of the URL messages entirely.
I left website_util in tact, but maybe that should be removed too, given it's no longer used.
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 12 May 2018 01:31:52 +0000
Gerrit-HasComments: No
[kudu-CR] remove logged Kudu URLs
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 16 May 2018 16:17:55 +0000
Gerrit-HasComments: No
[kudu-CR] util: fix logged Kudu URL
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
(2 comments)
> Patch Set 3:
>
> (2 comments)
>
> > This is only an issue in development while working on unreleased
> > code right? Even with this fix there are no docs deployed to for
> > that version yet right?
>
> Also, if we move to publishing the website from master, say, nightly (and I think we should), don't we want to include the -SNAPSHOT?
It's not just for dev builds. What prompted the change was that I saw a production cluster point to http://.../1.7.0-cdh5.12/docs
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc
File src/kudu/util/version_info.cc:
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@25
PS3, Line 25: #include <regex.h>
> C++11 has regex support now! I wrote up a quick example of matching version
I was basing the implementation on that in rpc/sasl_common.cc. A comment there mentioned it's broken in older versions of libstdcxx? Will look into it more.
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@49
PS3, Line 49: "\\([[:digit:]]\\+\\.[[:digit:]]\\+\\.[[:digit:]]\\+\\)-.*"
> This regex doesn't look right to me. Don't we want
I agree, I'm surprised it needed this much escaping, but in test, this is what worked.
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 20:04:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] util: fix logged Kudu URL
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
I wonder if it's worth embedding a specific version into our release at all. It sounds like it could be fragile and not very future proof.
Perhaps we could build a versions landing page in our docs that provides
a list of versions to choose from and point to that. (Maybe including nightly/dev in the future)
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 21:25:18 +0000
Gerrit-HasComments: No
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9280
to look at the new patch set (#7).
Change subject: remove logged Kudu URLs
......................................................................
remove logged Kudu URLs
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0/docs/...
...to link to pages for common-enough problems with complicated fixes
that warranted some documentation. Given how fragile these URLs can be,
this patch removes such messages, particularly since the URLs are
generated based on `version.txt`, which doesn't necessarily reflect a
proper URL (e.g. if on a SNAPSHOT build) and may be changed by
thirdparty vendors of Kudu.
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 2 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/7
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] util: fix logged Kudu URL
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
(2 comments)
> This is only an issue in development while working on unreleased
> code right? Even with this fix there are no docs deployed to for
> that version yet right?
Also, if we move to publishing the website from master, say, nightly (and I think we should), don't we want to include the -SNAPSHOT?
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc
File src/kudu/util/version_info.cc:
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@25
PS3, Line 25: #include <regex.h>
C++11 has regex support now! I wrote up a quick example of matching version strings and extracting the stripped part at https://gist.github.com/wdberkeley/cca372484b610501efe37cdeb08e0e7e.
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@49
PS3, Line 49: "\\([[:digit:]]\\+\\.[[:digit:]]\\+\\.[[:digit:]]\\+\\)-.*"
This regex doesn't look right to me. Don't we want
([[:digit:]]+.[[:digit:]]+.[[:digit:]]+)(-.*)?
with no escaping needed?
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 19:53:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9280
to look at the new patch set (#5).
Change subject: remove logged Kudu URLs
......................................................................
remove logged Kudu URLs
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0/docs/...
...to link to pages for common-enough problems with complicated fixes
that warranted some documentation. Given how fragile these URLs can be,
this patch removes such messages, particularly since the URLs are
generated based on `version.txt`, which doesn't necessarily reflect a
proper URL (e.g. if on a SNAPSHOT build) and may be changed by
thirdparty vendors of Kudu.
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 2 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/5
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9280
to look at the new patch set (#4).
Change subject: remove logged Kudu URLs
......................................................................
remove logged Kudu URLs
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0/docs/...
...to link to pages for common-enough problems with complicated fixes
that warranted some documentation. Given how fragile these URLs can be,
this patch removes such messages, particularly since the URLs are
generated based on `version.txt`, which doesn't necessarily reflect a
proper URL (e.g. if on a SNAPSHOT build) and may be changed by
thirdparty vendors of Kudu.
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 2 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/4
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] util: fix logged Kudu URL
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc
File src/kudu/util/version_info.cc:
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@25
PS3, Line 25: #include <regex.h>
> I was basing the implementation on that in rpc/sasl_common.cc. A comment th
Ah I see...if it's busted then we shouldn't use it. Todd wrote that comment, so maybe he can tell you what's busted.
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@49
PS3, Line 49: "\\([[:digit:]]\\+\\.[[:digit:]]\\+\\.[[:digit:]]\\+\\)-.*"
> I agree, I'm surprised it needed this much escaping, but in test, this is w
The crazy escaping may be the difference between the flavor of regex.h you're using and the default used in C++11 regex? Seems weird to me though. You also might be able to use C++11 raw strings to get rid of some escaping and make the regex more readable.
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/website_util.cc
File src/kudu/util/website_util.cc:
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/website_util.cc@37
PS3, Line 37: if (version_number.empty()) {
If I read the regex right, if the version is not of the form "x.y.z-something" then this will be empty- the "-something" on the end is not optional, though "something" could be empty. Therefore, won't we fail to give release URLs for e.g. 1.7.0?
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 20:26:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] remove logged Kudu URLs
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 16 May 2018 16:09:44 +0000
Gerrit-HasComments: No
[kudu-CR] util: fix logged Kudu URL
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
> It's not just for dev builds. What prompted the change was that I
> saw a production cluster point to http://.../1.7.0-cdh5.12/docs
Definitely we want to remove the -cdh mumbo jumbo, but don't we want to keep -SNAPSHOT? The link won't be any more broken now than a link to an unreleased version, and if we do publish the website from master, then I think we should have the -SNAPSHOT in the URL.
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 20:11:49 +0000
Gerrit-HasComments: No
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9280
to look at the new patch set (#6).
Change subject: remove logged Kudu URLs
......................................................................
remove logged Kudu URLs
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0/docs/...
...to link to pages for common-enough problems with complicated fixes
that warranted some documentation. Given how fragile these URLs can be,
this patch removes such messages, particularly since the URLs are
generated based on `version.txt`, which doesn't necessarily reflect a
proper URL (e.g. if on a SNAPSHOT build) and may be changed by
thirdparty vendors of Kudu.
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 2 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/6
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
remove logged Kudu URLs
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0/docs/...
...to link to pages for common-enough problems with complicated fixes
that warranted some documentation. Given how fragile these URLs can be,
this patch removes such messages, particularly since the URLs are
generated based on `version.txt`, which doesn't necessarily reflect a
proper URL (e.g. if on a SNAPSHOT build) and may be changed by
thirdparty vendors of Kudu.
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Reviewed-on: http://gerrit.cloudera.org:8080/9280
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/server/server_base.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 2 insertions(+), 8 deletions(-)
Approvals:
Grant Henke: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] util: fix logged Kudu URL
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9280
to look at the new patch set (#3).
Change subject: util: fix logged Kudu URL
......................................................................
util: fix logged Kudu URL
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0-SNAPSHOT/docs/...
Instead of:
http://kudu.apache.org/releases/1.7.0/docs/...
This patch parses a number out of the supplied version string. If a
version number can't be parsed out, a default URL will be used:
http://kudu.apache.org/docs/...
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M src/kudu/util/version_info.cc
M src/kudu/util/version_info.h
M src/kudu/util/website_util.cc
3 files changed, 29 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/3
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] remove logged Kudu URLs
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 14 May 2018 23:08:07 +0000
Gerrit-HasComments: No
[kudu-CR] remove logged Kudu URLs
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 16 May 2018 16:45:25 +0000
Gerrit-HasComments: No
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: remove logged Kudu URLs
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] util: fix logged Kudu URL
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9280
to look at the new patch set (#2).
Change subject: util: fix logged Kudu URL
......................................................................
util: fix logged Kudu URL
Previously we would log urls like:
http://kudu.apache.org/releases/1.7.0-SNAPSHOT/docs/...
Instead of:
http://kudu.apache.org/releases/1.7.0/docs/...
This patch parses a number out of the supplied version string. If a
version number can't be parsed out, a default URL will be used:
http://kudu.apache.org/docs/...
Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
---
M src/kudu/util/version_info.cc
M src/kudu/util/version_info.h
M src/kudu/util/website_util.cc
3 files changed, 29 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/9280/2
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] util: fix logged Kudu URL
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
> Patch Set 3:
>
> (3 comments)
>
> > Patch Set 3:
> >
> > > It's not just for dev builds. What prompted the change was that I
> > > saw a production cluster point to http://.../1.7.0-cdh5.12/docs
> >
> > Definitely we want to remove the -cdh mumbo jumbo, but don't we want to keep -SNAPSHOT? The link won't be any more broken now than a link to an unreleased version, and if we do publish the website from master, then I think we should have the -SNAPSHOT in the URL.
Maybe, I guess it'd be dependent on if/how we implement a /dev version for the site. E.g. it could be we just use the version number, but slap on a warning at the top of every page saying it's just a dev snapshot and may not reflect the release, etc. That said, it's worth considering leaving any extra parsing up to the user, if they so choose to change version.txt themselves.
Todd mentioned perhaps using s.apache.org, since it'd give us some flexibility, although I think going down that path would need some extra steps to how we build/release the site (maybe we should snatch s.apache.org/kudu_docs-X.Y.Z now? Although going down that root makes it a bit harder to link to specific pages)
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 21:08:08 +0000
Gerrit-HasComments: No
[kudu-CR] util: fix logged Kudu URL
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
(3 comments)
> Patch Set 3:
>
> > It's not just for dev builds. What prompted the change was that I
> > saw a production cluster point to http://.../1.7.0-cdh5.12/docs
>
> Definitely we want to remove the -cdh mumbo jumbo, but don't we want to keep -SNAPSHOT? The link won't be any more broken now than a link to an unreleased version, and if we do publish the website from master, then I think we should have the -SNAPSHOT in the URL.
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc
File src/kudu/util/version_info.cc:
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@25
PS3, Line 25: #include <regex.h>
> Ah I see...if it's busted then we shouldn't use it. Todd wrote that comment
Ack
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/version_info.cc@49
PS3, Line 49: "\\([[:digit:]]\\+\\.[[:digit:]]\\+\\.[[:digit:]]\\+\\)-.*"
> The crazy escaping may be the difference between the flavor of regex.h you'
Yeah, it really is weird. Even basing it on rpc/sasl_common.cc, I was expecting a different expression. Probably easier to just use std::regex
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/website_util.cc
File src/kudu/util/website_util.cc:
http://gerrit.cloudera.org:8080/#/c/9280/3/src/kudu/util/website_util.cc@37
PS3, Line 37: if (version_number.empty()) {
> If I read the regex right, if the version is not of the form "x.y.z-somethi
Yep, just noticed this too.
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 20:34:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] util: fix logged Kudu URL
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: util: fix logged Kudu URL
......................................................................
Patch Set 3:
This is only an issue in development while working on unreleased code right? Even with this fix there are no docs deployed to for that version yet right?
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Feb 2018 14:54:00 +0000
Gerrit-HasComments: No
[kudu-CR] remove logged Kudu URLs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9280 )
Change subject: remove logged Kudu URLs
......................................................................
Patch Set 5: Verified+1
Failure seems unrelated.
--
To view, visit http://gerrit.cloudera.org:8080/9280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I357693122d27337183cb85c677a85e7b8d63fe48
Gerrit-Change-Number: 9280
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 12 May 2018 02:03:40 +0000
Gerrit-HasComments: No