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

1.0.5 #197

Merged
merged 18 commits into from
Jan 16, 2024
Merged

1.0.5 #197

merged 18 commits into from
Jan 16, 2024

Conversation

@eeintech
Copy link
Contributor Author

@T0jan I think your PR has introduced something weird as alternate resistors are created as new parts in InvenTree when they shouldn't, I didn't get much time to look into it but I can't reproduce it within the GUI. Any ideas?

@T0jan
Copy link
Collaborator

T0jan commented Jan 15, 2024

@eeintech I haven't changed anything involved with the alternate test so it has to be some kind of side effect. lets see if I can find anything. It is very weird, that it works for the capacitor but not for the resistors later, so maybe it is something with the configs, not the code.

@T0jan
Copy link
Collaborator

T0jan commented Jan 15, 2024

@eeintech Ok I found the reason: it is the Temperature Range parameter. As we use the stable branch again the parameter check is active by default (#165) and so the parameter gets rejected. For resistors it is part of the compared parameters, for capacitors not so this explains why they act different.

I don't know if it is possible to change the inventree config during the test or in the startup to disable the parameter check.

@eeintech
Copy link
Contributor Author

@T0jan This is strange, I cannot reproduce the Temperature Range discrepancy locally, although enabled in my paramaters_filters.yaml config file:

image

Both original and alternate MPNs show those parameters locally:

{'Package Type': '0603', 'Rated Power': '1/10W', 'Temperature Range': '-55~155°C', 'Tolerance': '±5%', 'Value': '10K'}

@T0jan
Copy link
Collaborator

T0jan commented Jan 15, 2024

@eeintech has the local server you test this with the parameter test enabled? or do you just run the release pipeline locally?

I run the jobs in my fork with debug output and there the discrepancy is clear:

https://github.com/T0jan/Ki-nTree/actions/runs/7529280107/job/20493179309#step:10:1763

@eeintech
Copy link
Contributor Author

@T0jan It seems like there is now some kind of conversion going on with parameters?

requests.exceptions.HTTPError: {'detail': 'Error occurred during API request', 'url': 'http://127.0.0.1:8000/api/part/parameter/', 'method': 'POST', 'status_code': 400, 'body': '{"data":["Could not convert -55~155°C to °C"]}', 'headers': {'AUTHORIZATION': 'Token inv-b344cee97aceb609bee46f5beab6aced190a3f7e-20240115'}, 'params': {'format': 'json'}, 'data': {'part': 1, 'template': 50, 'data': '-55~155°C'}}

The devil must be in the details because it works on local setup, which means that °C is not equal to °C??

@T0jan
Copy link
Collaborator

T0jan commented Jan 15, 2024

@eeintech yes there is a conversion, it is the #163 thing which they added for 0.12.0. Inventree by default expects now parameters to be SI-compatible so always something like -50°C, 100mm, 42uF and so on. So a multivalue entry like -55~155°C is not allowed by the server. I told them this is not the best idea so they added a button to be able to switch this off, do you remember?

image

I do not know if it is possible to unset this setting via environmental variables in the server setup.

Also I noticed that my check for exact this setting does not work anymore as they changed the error message. So this is fun...

@eeintech
Copy link
Contributor Author

eeintech commented Jan 15, 2024

@T0jan I'm sorry, this is new setting completely came out of my mind... just too many things going on right now 😓

I do not know if it is possible to unset this setting via environmental variables in the server setup.

Righto I need to follow-up on that... I'm tempted to merge this right now and release 1.0.5 with all the changes then sort this out.
If that setting is enabled by default on new and older InvenTree setups, it will cause duplicate resistors to be created (eg. rendering the "Check Existing" toggle useless). What do you think?

@T0jan
Copy link
Collaborator

T0jan commented Jan 15, 2024

@eeintech No worries, I should have written it clear right away, would have saved you the debug time...

I don't think anything speaks against releasing it as is per se, as the setting is documented already it should not cause to much problems for users and not a problem with Ki-nTree. However I would like to patch the error detection for this specific case, so the user gets a clear message if they did not disabled this setting.

If that setting is enabled by default on new and older InvenTree setups, it will cause duplicate resistors to be created (eg. rendering the "Check Existing" toggle useless). What do you think?

I don't think so. It only gets useless if the setting is not deactivated and the part parameters include a non convertible value. The default case should be the deactivated case where all parameters are generated as before (That's why we added it to the documentation) and there the existing part check will work as before.

@eeintech eeintech merged commit 6318053 into main Jan 16, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

Error handling when adding parameters
4 participants