Octopart v4 API #1150
Merged
Octopart v4 API #1150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'd like to see it merged. Is it still WIP? |
|
I thought I'd the time to finish the document import... Maybe later. Please go on with the merge. |
|
Hey, this sounds neat! I was bugging out recently with the apparent broken octopart. Most changes seem pretty straightforward, I'm wondering a bit about the predisclient though? I read that predis is php7.2, but we are stuck on 7.0-7.1 atm. How are you using (p)redis here exactly? (as a kind of cache for the octopart api calls?) |
|
I used the docker image mhubig/partkeepr to develop this PR, which uses php-7.1. The predis is already part of PartKeepr's dependencies and seems to work just fine. I'm caching the query answers to import a certain device without requesting it again (to reduce the number of api calls, which are limited per month). It should work fine without a redis server, but will then require more api calls. |
|
@sibbi77 Case sizes seem to be dropping leading "0" so 0402 becomes 402, looks like they are being stored as numeric rather than text. |
|
0402 is now considered a string in the fields "Case/Package", "Case Code (Imperial)" and "Case Code (Metric)". |
|
@sibbi77 Just tried this but it made no difference, the git log shows your changes but it's still the same, I did a pull then setup. |
|
be sure to execute |
|
I did that and now case sizes and prefixes seem ok now, But parameters seem to not be completely consistent with the past API, So far I have noticed resistor tolerance as below |
|
Definitely needs some better importing. And seems some values are outright missing? |
|
Octopart has complete rewritten the API. The former unit concept is no longer available, only a display value (e.g. 100 µm) is provided and I try to recognize the value, unit, and SI prefix. |
|
have a look at https://octopart.com/mc00625w04021147k-multicomp-30314144?r=sp |
|
It does not work for me
|
|
OK I had to change plan to free PRO and it works. Thank you! @edit, no it doesn't work, when I click "Add data" I'm getting error: |
|
which part do you try to add? |
|
@sibbi77 for example this one |
|
I found a minor bug, if the search returns no results it just says "loading" forever. |
|
I just tried to add the LM317T and had no problems with it. So not reproducible for me. |
|
@chrissnow I tried with the item "unknownitem" and got an error dialog (not with "no results returned", but with an error message). |
|
@chrissnow what browser? There is no datasheets field so it fail |
|
you are not using the current PR codebase. The code you show is commented. |
|
@sibbi77 I get this |
|
@sibbi77 ok, it is working, I had to re-run the setup, probably some compiled js was not compiled, I don't know why but it works now. Thanks |
|
@sibbi77 part: MC34063ACN |
|
the data from Octopart is erronous see here: |
| @@ -152,15 +158,36 @@ Ext.define("PartKeepr.Components.OctoPart.DataApplicator", { | |||
| } | |||
| } | |||
|
|
|||
| if (this.import.cadModels) | |||
| /* if (this.import.datasheets) | |||
There was a problem hiding this comment.
You might want to remove this obsolete code fragment.
| @@ -170,7 +197,7 @@ Ext.define("PartKeepr.Components.OctoPart.DataApplicator", { | |||
| } | |||
| } | |||
|
|
|||
| if (this.import.complianceDocuments) | |||
| /* if (this.import.complianceDocuments) | |||
There was a problem hiding this comment.
Here, too, you could remove this block comment.
|
@dromer I think this looks good. Multiple users have confirmed that the PR seems to work. Our current solution does indeet not work. So I see not much harm in merging this, although not having tested and reviewed it thoroughly (PHP part looks good, JS I cannot say much, the requests made I did not look up in detail). So I vote for merging sooner than later. (Maybe after the two small code changes I suggested, if @sibbi77 is ok with them). |
|
@sibbi77 there is a bug, when I search for something and there is no results (just try "sdfsfdsfd") then javascript exception is being thrown in the console, octopart search is not working afterwards |
Codecov Report
@@ Coverage Diff @@
## master #1150 +/- ##
============================================
- Coverage 37.09% 36.87% -0.22%
- Complexity 1798 1803 +5
============================================
Files 258 258
Lines 5729 5762 +33
============================================
Hits 2125 2125
- Misses 3604 3637 +33
Continue to review full report at Codecov.
|
|
@piotrkochan I get the same as in #1150 (comment) and the search works after that. |
|
This breaks the demo system: |
|
I added |
|
It's a manually maintained file. I wasn't aware of that the parameter was added, I'll add it to the config file for the demo. |
|
@Drachenkaetzchen the page at https://demo-git.partkeepr.org has a wrong config set up currently. There is a message about a wrong username/password. Could you please recheck the config file? |
It is a result of the git pull request, I haven't had time to implement the new config. |
|
Wrong username/password always occurs if something goes wrong with the scripts creating the demo site. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the existing Octopart component for the new API.
It optionally uses a local redis server to cache the result set from Octopart.
What does not work: