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

Use with the new "Release 2" API #7

Open
bohanyang opened this issue Apr 19, 2019 · 9 comments
Open

Use with the new "Release 2" API #7

bohanyang opened this issue Apr 19, 2019 · 9 comments

Comments

@bohanyang
Copy link

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.

@drupol
Copy link

drupol commented Jun 14, 2019

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 ?

@autaut03
Copy link

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.

@drupol
Copy link

drupol commented Aug 19, 2019

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.

@autaut03
Copy link

@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 [5.0] Environment variables sometimes return null laravel/framework#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.

@drupol
Copy link

drupol commented Aug 19, 2019

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 ?

@autaut03
Copy link

@drupol

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

dev-master looks good. Thanks again.

@drupol
Copy link

drupol commented Aug 22, 2019

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

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

@drupol
Copy link

drupol commented Aug 22, 2019

Release 2.1.0 is available!

@autaut03
Copy link

Release 2.1.0 is available!

Thank you!

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