You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by "mraygalaxy2 (via GitHub)" <gi...@apache.org> on 2023/08/25 15:24:48 UTC

[GitHub] [libcloud] mraygalaxy2 opened a new pull request, #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

mraygalaxy2 opened a new pull request, #1946:
URL: https://github.com/apache/libcloud/pull/1946

   ## Updated Linode (Akamai Connected Cloud) support (including cloud-init)
   
   ### Description
   
   1. cloud-init support is a new feature available in 2023.
   2. The main entry point (create_node) had an arrangement of non-keyword parameters that were not consistent with other libcloud drivers. This has been fixed.
   3. One remaining function (already available in the API) was exposed to also be consistent with other drivers.
   
   ### Status
   
   - done, ready for review
   
   ### Checklist (tick everything that applies)
   
   - [ ] [Code linting](http://libcloud.readthedocs.org/en/latest/development.html#code-style-guide) (required, can be done after the PR checks)
   - [ ] Documentation
   - [ ] [Tests](http://libcloud.readthedocs.org/en/latest/testing.html)
   - [ ] [ICLA](http://libcloud.readthedocs.org/en/latest/development.html#contributing-bigger-changes) (required for bigger changes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1718155382

   @Kami Thank you, sir. I'll push a change right now to update that document.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1694712444

   Thanks for the contribution.
   
   When you get a chance, can you please add some test case for those changes? Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1728231724

   Thanks for addressing the review feedback.
   
   I made some small changes and merged it into trunk:
   
   * Reword upgrade notes entry a bit - 01c7e61de89144205a2dcc1db31239bb527b725c
   * Default `ex_userdata` to `None` instead of `False` since this makes more sense for an argument which is not a boolean - bb0dba7d0fdc24bf4b21209a6e4267dd6c773a6d
   * Add test case for `ex_userdata` argument - bb0dba7d0fdc24bf4b21209a6e4267dd6c773a6d


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1718152962

   @mraygalaxy2 Thanks for adding the tests and addressing some very old API inconsistency in the ``create_node()`` method.
   
   I think it would be good to document that breaking API change in the upgrade notes file (https://github.com/apache/libcloud/commit/d8add44eec876cae1103b99d545a747a0e33a4bc#r127271125).
   
   Besides that, LGTM.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1718158273

   @mraygalaxy2 Thanks. There is one more small thing which I thought I already commented on in the past (https://github.com/apache/libcloud/pull/1946/files#r1324941487), but maybe I forgot to :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on a diff in pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on code in PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#discussion_r1328961676


##########
libcloud/compute/drivers/linode.py:
##########
@@ -997,6 +1038,12 @@ def create_node(
         :keyword ex_private_ip: whether or not to request a private IP
         :type    ex_private_ip: ``bool``
 
+        :keyword ex_userdata: add cloud-config compatible userdata to be

Review Comment:
   okie dokie @Kami @Dorthu  I have pushed that change (and tested it against Linode).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1695766290

   Acknowledged @Kami . I'll work on that.
   
   Regarding the name change, we haven't been directed to do that right now by the company, particularly as the API endpoint (api.linode.com) is unable to change for the forseeable future.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1723821968

   Also tell me if those those docs/upgrade_notes.rst and CHANGES.rst changes look ok. I'm happy to clean them up if there's a mistake.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Dorthu commented on a diff in pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Dorthu (via GitHub)" <gi...@apache.org>.
Dorthu commented on code in PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#discussion_r1325819055


##########
libcloud/compute/drivers/linode.py:
##########
@@ -997,6 +1038,12 @@ def create_node(
         :keyword ex_private_ip: whether or not to request a private IP
         :type    ex_private_ip: ``bool``
 
+        :keyword ex_userdata: add cloud-config compatible userdata to be

Review Comment:
   I agree that it would be more convenient for the library to handle the encoding, especially if there's precedent elsewhere in libcloud, as long as that behavior is featured prominently in the documentation. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on a diff in pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on code in PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#discussion_r1324941487


##########
libcloud/compute/drivers/linode.py:
##########
@@ -997,6 +1038,12 @@ def create_node(
         :keyword ex_private_ip: whether or not to request a private IP
         :type    ex_private_ip: ``bool``
 
+        :keyword ex_userdata: add cloud-config compatible userdata to be

Review Comment:
   
   
   I think it would actually be better to let user pass in a raw string and then we can take care of base64 encoding the string inside the method ourselves.
   
   This way the method abstracts away this implementation detail from the end user (we already do a similar thing in a bunch of places).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1728351810

   Thank you so much, @Kami for the changes! Much appreciated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1694713040

   It also seems like that it may be a good idea to rename the driver in the future (Akamai Connected Cloud)?
   
   We can still leave (deprecated) "linode" alias in place for the foreseeable future to make the change backward compatible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1701330448

   okie dokie @Kami  I've pushed those tests. Take a 2nd look?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#issuecomment-1701370984

   NOTE: I do want to remove (or cleanup) this comment: https://github.com/apache/libcloud/pull/1946/commits/d8add44eec876cae1103b99d545a747a0e33a4bc#diff-e2e26eb0ef50c4cf2bf6b12658cd644d7ca1fe06cd2a3bc307d607149c83a2a4R976
   
   Looking for feedback before I do that.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] asfgit merged pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit merged PR #1946:
URL: https://github.com/apache/libcloud/pull/1946


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] Kami commented on a diff in pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on code in PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#discussion_r1324937049


##########
libcloud/compute/drivers/linode.py:
##########
@@ -945,15 +973,28 @@ def reboot_node(self, node):
     def create_node(
         self,
         location,
-        size,
-        image=None,
-        name=None,
+        # Previously, the following 3 parameters did not match the rest of the libcloud
+        # codebase drivers. They should be in the same order as other compute drivers.
+        # Previously, it looked like this:
+        #         size,
+        #         image=None,
+        #         name=None,
+        #
+        # Comments welcome on how backwards compatibility (if any) should work here.

Review Comment:
   I think it's fine to leave it as-in and document this backward incompatible / breaking change in the upgrade notes file (https://github.com/apache/libcloud/blob/trunk/docs/upgrade_notes.rst), ideally with some concrete before / after code examples.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on a diff in pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on code in PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#discussion_r1324944258


##########
libcloud/compute/drivers/linode.py:
##########
@@ -997,6 +1038,12 @@ def create_node(
         :keyword ex_private_ip: whether or not to request a private IP
         :type    ex_private_ip: ``bool``
 
+        :keyword ex_userdata: add cloud-config compatible userdata to be

Review Comment:
   @Dorthu Would you like me to make that change? ^^^



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [libcloud] mraygalaxy2 commented on a diff in pull request #1946: Updated Linode (Akamai Connected Cloud) support (including cloud-init)

Posted by "mraygalaxy2 (via GitHub)" <gi...@apache.org>.
mraygalaxy2 commented on code in PR #1946:
URL: https://github.com/apache/libcloud/pull/1946#discussion_r1326107629


##########
libcloud/compute/drivers/linode.py:
##########
@@ -997,6 +1038,12 @@ def create_node(
         :keyword ex_private_ip: whether or not to request a private IP
         :type    ex_private_ip: ``bool``
 
+        :keyword ex_userdata: add cloud-config compatible userdata to be

Review Comment:
   Will do! Thanks guys.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org