New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LIBCLOUD-547] linode.py labels too long with name use linode id instead #287

Closed
wants to merge 7 commits into
base: trunk
from

Conversation

Projects
None yet
2 participants
@earthgecko
Contributor

earthgecko commented Apr 29, 2014

Using linode["id"] for labels and the labels step needed to be moved down to after the linode object exists so that the id can be used.

Apologies about the old commit messages could not get rid of them rebased and all but the diff is sound.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami May 2, 2014

Member

Sorry for the delay, I'll review the pull request now.

Member

Kami commented May 2, 2014

Sorry for the delay, I'll review the pull request now.

# Use the linode id as the name can be up to 63 chars and the labels
# are limited to 48 chars
label = {
"lconfig": "[%s] Configuration Profile" % linode["id"],

This comment has been minimized.

@Kami

Kami May 2, 2014

Member

This looks good, although if we wanted to be sure that labels are never longer we could truncate it in this case (`...[:48]).

@Kami

Kami May 2, 2014

Member

This looks good, although if we wanted to be sure that labels are never longer we could truncate it in this case (`...[:48]).

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami May 2, 2014

Member

@earthgecko I wanted to apply your patch into trunk, but I'm getting conflicts.

Can you please sync you branch with upstream trunk and squash all the commits?

Member

Kami commented May 2, 2014

@earthgecko I wanted to apply your patch into trunk, but I'm getting conflicts.

Can you please sync you branch with upstream trunk and squash all the commits?

@asfgit asfgit closed this in 940e10d May 6, 2014

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami May 7, 2014

Member

I went ahead and manually merged the changes into trunk. Thanks.

Member

Kami commented May 7, 2014

I went ahead and manually merged the changes into trunk. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment