Skip to content
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

api version 2 #2

Open
kablamo opened this issue Oct 30, 2014 · 10 comments
Open

api version 2 #2

kablamo opened this issue Oct 30, 2014 · 10 comments

Comments

@kablamo
Copy link

kablamo commented Oct 30, 2014

Hi @Getty,

I like what you've done with this module. Are you interested in a pr which adds to the start you've made on version 2 of the api?

@Getty
Copy link
Owner

Getty commented Oct 30, 2014

Yeah sure, anything that works is welcome, i dont use the module right now, but will probably use it again someday ;) but i can add you as contributor after the pull request

@kablamo
Copy link
Author

kablamo commented Oct 30, 2014

Sounds good!

How do you feel about losing support for v1.2? Because

  • v2.0 has been out for about 3 years.
  • It would make the code simpler.
  • They turned off v1.2 in the sandbox which makes it hard to test I haven't broken things. Its still running in production, but I'd rather not run tests against my production env.
  • I'm kind of whining/making excuses, but if it were up to me, I'd just abandon v1.2.
  • I would try to keep the UI the same so its still backwards compatible.

@Getty
Copy link
Owner

Getty commented Oct 31, 2014

yeah but dropping out stuff that might be used by someone still sounds not like really required, if you have specific problems with the implementation in manner to API v1.2 i can help to make it slim again ;) , but in general i prefer to keep "thing that work" in to make all people happy :). Hope you can be with me on that ;) but we can slim it up if you want, i never actually looked into API v2.0 and i might can optimize it out for you that you dont see the legacy anymore ;). What specific is it that annoys you on this path you are up to with keeping the 1.2?

@kablamo
Copy link
Author

kablamo commented Oct 31, 2014

Hmmm. I agree we should keep backwards compatibility. :) Old code that uses WWW::DNSMadeEasy should keep working. But if I keep the module's user interface backwards compatible, does it matter if v1.2 or v2.0 is happening underneath?

If you want to keep v1.2, I'm happy to do that. I'm just trying to figure out what makes sense.

@Getty
Copy link
Owner

Getty commented Oct 31, 2014

We have an attribute for the version and the API can adapt, like disallow calls for specific version, or even change the API of them depending on the case. Thats the beauty of Perl "anything goes". ;)

@Getty
Copy link
Owner

Getty commented Oct 31, 2014

If you want to change the classes you can make V2 classes, and stuff.

@markstos
Copy link

I'm also a DNSME user and appreciate the work. Some feedback on the v2 effort:

  • I recommend considering using Role::REST::Client internally. It's a Moo role that makes it easy and pleasant to create Perl modules based on REST APIs like this.
  • Records should be first-class objects. You shouldn't have go through a "domain" object to get one, as the docs present it now. I should be able to express "I want to see the details for the "subdomain.example.com" record. This was unclear with the v1.2 client and it remains unclear with the v2.0 client. Perhaps this can be done with method chaining and just needs clearer documentation.

@kablamo
Copy link
Author

kablamo commented Feb 24, 2015

  • Role::REST::Client looks really nice. I wish I had known about it before writing my pr.
  • The current code allows you to do this:
my ($record) = $dme->get_managed_domain('example.com')
    ->records(name => 'subdomain');
say $record->ttl;

Perhaps there could be a shortcut for that so its just one function call. I think thats what you are saying. That sounds like a good idea.

Appreciate the feedback @markstos !

@markstos
Copy link

I used Role::REST::Client for a few projects. It was indeed pleasant to use.

The method chain shown is workable, but this would be a welcome shortcut:

my $record = $dme->get_record(
   domain => 'example.com',
  %other_args_to_records
);

If the domain ID lookup was cached in memory, then follow-up calls to get_record() would only make additional network requests to look up the domain details if the domain had not been seen before.

@kablamo
Copy link
Author

kablamo commented Feb 24, 2015

@markstos 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants