v1.4.1 Fix custom header config/params not applied to API request #24
Conversation
There was a problem hiding this comment.
@Xaxxis adding enhancement to this bugfix is not a good idea. Please fix this PR or re-open PR so it contains the bugfix parts only and with v1.4.1
Will explain why later.
|
The bugfix parts looks good (approved) ✅ , so once you have PR for bugfix parts only, you can go ahead merge & release. |
Some of the reasons:
So that's why better release the bugfix first as v1.4.1 , then we can take time with the v1.5 enhancement. Bug fix is more prioritized. |
midtransclient/http_client.py
Outdated
| message='The ServerKey is invalid, as it is an empty string or Null. Please double-check your API key. You can check the ServerKey from the Midtrans Dashboard. See https://docs.midtrans.com/en/midtrans-account/overview?id=retrieving-api-access-keys for the details or contact support at support@midtrans.com if you have any questions.', | ||
| api_response_dict=None, |
There was a problem hiding this comment.
Review for the enhancement part (to address later after bugfix release):
- I think Bintar's team is starting to deprecate email support in favor of website contact form. Should replace the email with URL instead.
There was a problem hiding this comment.
Agreed, sure will remove the email
midtransclient/http_client.py
Outdated
| if " " in server_key: | ||
| raise MidtransAPIError( | ||
| message='The ServerKey is contains white-space. Please double-check your API key. You can check the ServerKey from the Midtrans Dashboard. See https://docs.midtrans.com/en/midtrans-account/overview?id=retrieving-api-access-keys for the details or contact support at support@midtrans.com if you have any questions.', | ||
| api_response_dict=None, | ||
| http_status_code=0, | ||
| raw_http_client_data=None | ||
| ) | ||
|
|
There was a problem hiding this comment.
Grammar issue, no need is.
Btw this is my opinion, but feel free to decide. I don't think we should check for whitespace or any specific chars, because the Spec can change from API side. And if it changed, we will need to edit this kind of logic manually each time. Also I think the API raw error message in this scenario already tell merchant that ServerKey is invalid.
So IMO the benefit is small & the potential maintenance burden is higher.
There was a problem hiding this comment.
Sure will remove this part. Thanks da
|
@Xaxxis I will change/revert this PR into the fixed part ya. Thanks |
|
Done the PR revert to Fix bug only ya @rizdaprasetya |
Xaxxis
changed the title
|
Great thanks @Xaxxis, merged. Please try to continue to release ya, let me know if any issue. |
rizdaprasetya
changed the title
Bug Fix