randomorg icon indicating copy to clipboard operation
randomorg copied to clipboard

Use with the new "Release 2" API

Open bohanyang opened this issue 5 years ago • 9 comments

Since we can only create developer key of the new "Release 2" API now, if you are trying to use this package with it, you have to update the API endpoint as following:

$random = new RandomOrg\Random(
  'testtest-test-test-test-testtesttest',
  new RandomOrg\Client(
    'https://api.random.org/json-rpc/2/invoke'
  )
);

Hope this can help. Also, please consider to update the default API endpoint. Thanks.

bohanyang avatar Apr 19 '19 16:04 bohanyang

Hi,

https://github.com/drupol/yaroc can use API V1 or V2... maybe it would be a good idea to merge both of these projects ?

drupol avatar Jun 14 '19 11:06 drupol

Hi,

https://github.com/drupol/yaroc can use API V1 or V2... maybe it would be a good idea to merge both of these projects ?

Hey. I don't think it will ever get merged unless these lines are completely removed: https://github.com/drupol/yaroc/blob/master/src/RandomOrgAPI.php#L70

I personally don't want that in my production code.

autaut03 avatar Aug 19 '19 13:08 autaut03

Hi @autaut03 ,

Thanks for your feedback.

I'm willing to please the user of this library, but I have to say that I don't understand why that. Could you please explain why you don't want that ? I guess it's for security reason, but I would like to know more. What is dangerous about that ? Do you see a better alternative (besides loading the api key via the constructor)

Thanks.

drupol avatar Aug 19 '19 13:08 drupol

@drupol

Hey again. There are two reasons why I wouldn't want that in my code:

  1. I don't want some files to be opened with each request. I use opcache on all of my production/staging environments so there's no need to be loading .env. Moreover, forcefully loading .env values might break my auto-deploy, as it relies on config (which utilitizes values from .env) being cached (with old values) while deploying. Furthermore, opening .env with each request on production will throw an exception sometimes (see https://github.com/laravel/framework/issues/8191) due to a bug.
  2. It's outside of your library's scope. It's job is to provide an API client, nothing more. It's generally just a bad practice.

A better alternative is constructor or setter only, both of which are currently implemented. Those are great options.

autaut03 avatar Aug 19 '19 17:08 autaut03

Fair enough. Totally make sense.

I wasn't seeing this like this, but you're right, it's not up to the library to do that.

I just pushed some commits in order to move symfony/dotenv from require to require-dev to ensure that tests are passing on Travis and locally.

Would you like to provide some feedback on the dev-master branch ?

drupol avatar Aug 19 '19 20:08 drupol

@drupol

Sorry for a late response. I'm very grateful for your quick response and understanding :)

dev-master looks good. Thanks again.

autaut03 avatar Aug 22 '19 18:08 autaut03

No worry, comments are async, so it's fine.

I will prepare a new release and ping you here when it's done.

drupol avatar Aug 22 '19 19:08 drupol

Release 2.1.0 is available!

drupol avatar Aug 22 '19 19:08 drupol

Release 2.1.0 is available!

Thank you!

autaut03 avatar Aug 24 '19 18:08 autaut03