You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ygy <gi...@git.apache.org> on 2015/08/26 17:32:28 UTC

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

GitHub user ygy opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/868

    Adding timestamp to the template options' name

    - timeStamp replaces the previously used randId. 
    - timeStamp uses the standard unix timestamp represented
      as a 8-char hex string.
    - It represents the moment in time when the name is constructed. 
    - It gives the possibility to search easily for instances, security
      groups, keypairs, etc based on timestamp without complicated
      enumeration

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ygy/incubator-brooklyn feature/embed-timestamp-to

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/868.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #868
    
----
commit cdcce8c6cb738eeae24dbb89b8f911e192cdb3e8
Author: Yavor Yanchev <ya...@yanchev.com>
Date:   2015-08-26T15:29:35Z

    Adding timestamp to the template options' name
    
    - timeStamp replaces the previously used randId. 
    - timeStamp uses the standard unix timestamp represented
      as a 8-char hex string.
    - It represents the moment in time when the name is constructed. 
    - It gives the possibility to search easily for instances, security
      groups, keypairs, etc based on timestamp without complicated
      enumeration

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by ygy <gi...@git.apache.org>.
Github user ygy commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#issuecomment-135666023
  
    It will be highly unlikely to get 2 machines with one and the same name. 
    The name is comprised by several other parameters like `appId` and `entityId`. They are random strings as well.
    Most importantly jclouds itself append a randomly generated 8-char suffix at the end.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by ygy <gi...@git.apache.org>.
Github user ygy commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#discussion_r38178763
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/location/cloud/names/BasicCloudMachineNamer.java ---
    @@ -41,9 +41,15 @@ protected String generateNewIdOfLength(ConfigBag setup, int len) {
             StringShortener shortener = Strings.shortener().separator("-");
             shortener.append("system", "brooklyn");
             
    -        // randId often not necessary, as an 8-char hex identifier is added later (in jclouds? can we override?)
    -        // however it can be useful to have this early in the string, to prevent collisions in places where it is abbreviated 
    -        shortener.append("randId", Identifiers.makeRandomId(4));
    +        /* timeStamp replaces the previously used randId. 
    +         * 
    +         * timeStamp uses the standard unix timestamp represented as a 8-char hex string.
    +         * 
    +         * It represents the moment in time when the name is constructed. 
    +         * It gives the possibility to search easily for instances, security groups, keypairs, etc
    +         * based on timestamp without complicated enumeration        
    +         */ 
    +        shortener.append("timeStamp", Long.toString(System.currentTimeMillis() / 1000L, 16));
    --- End diff --
    
    Thanks @bostko 
    Addressed in commit 7fff655b05df098d9f35c97863da0d9902bf420a


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/868


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by CMoH <gi...@git.apache.org>.
Github user CMoH commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#issuecomment-135667626
  
    I see - thanks for clarifying this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#issuecomment-135673908
  
    @ygy the build is failing at `CloudMachineNamerTest.testLengthMaxPermittedForMachineName` (see https://builds.apache.org/job/incubator-brooklyn-pull-requests/org.apache.brooklyn$brooklyn-core/1746/testReport/junit/org.apache.brooklyn.core.location.cloud/CloudMachineNamerTest/testLengthMaxPermittedForMachineName/). Can you take a look at fixing that please?
    
        java.lang.AssertionError: expected [10] but found [9]
        	at org.testng.Assert.fail(Assert.java:94)
        	at org.testng.Assert.failNotEquals(Assert.java:494)
        	at org.testng.Assert.assertEquals(Assert.java:123)
        	at org.testng.Assert.assertEquals(Assert.java:370)
        	at org.testng.Assert.assertEquals(Assert.java:380)
        	at org.apache.brooklyn.core.location.cloud.CloudMachineNamerTest.testLengthMaxPermittedForMachineName(CloudMachineNamerTest.java:118)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#discussion_r38092222
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/location/cloud/names/BasicCloudMachineNamer.java ---
    @@ -41,9 +41,15 @@ protected String generateNewIdOfLength(ConfigBag setup, int len) {
             StringShortener shortener = Strings.shortener().separator("-");
             shortener.append("system", "brooklyn");
             
    -        // randId often not necessary, as an 8-char hex identifier is added later (in jclouds? can we override?)
    -        // however it can be useful to have this early in the string, to prevent collisions in places where it is abbreviated 
    -        shortener.append("randId", Identifiers.makeRandomId(4));
    +        /* timeStamp replaces the previously used randId. 
    +         * 
    +         * timeStamp uses the standard unix timestamp represented as a 8-char hex string.
    +         * 
    +         * It represents the moment in time when the name is constructed. 
    +         * It gives the possibility to search easily for instances, security groups, keypairs, etc
    +         * based on timestamp without complicated enumeration        
    +         */ 
    +        shortener.append("timeStamp", Long.toString(System.currentTimeMillis() / 1000L, 16));
    --- End diff --
    
    You could use Long.toString(1440680387, Character.MAX_RADIX) or even more explicitly Long.toString(l, 36)
    This narrows down the required string length for the timestamp to 6


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#issuecomment-135739710
  
    Thanks @ygy - merging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adding timestamp to the template ...

Posted by CMoH <gi...@git.apache.org>.
Github user CMoH commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/868#issuecomment-135656129
  
    I might be asking the wrong question here, but what will happen if more machines are created within the same second? Will they get the same name? Would this be a problem or not?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---