-
Notifications
You must be signed in to change notification settings - Fork 0
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
#68 fixing import directives #69
Conversation
Sealdolphin
commented
Jun 22, 2022
- Changed name of import libraries
source/incqueryserver-jupyter/iqs_jupyter/analysis_extensions.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I have no idea what the problem is, I cannot judge how well it is being solved here.
if 'MmsRepositoryApi' in dir(iqs_client): | ||
from iqs_jupyter.mms_extensions import * | ||
from iqs_jupyter.mms_direct_extensions import * | ||
#if 'MmsRepositoryApi' in dir(iqs_client): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that we no longer ship trimmed down versions of the core API, but is there any specific need to remove these conditionals? I would expect them to work just fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found it inconsistent, because the new generator creates a different architecture... If we want it to stay, we may change it, but I thought it was easier for now to omit these features. This is a work in progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we changed which openapi generator we use? Is there a specific reason why we instituted such a change? Does it otherwise generate better client code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the new version is generated using a gradle plugin, instead of a library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I mean OpenAPI (whether invoked from the CLI or via Gradle) has a selection of plugins called "generators". Are we still using the same one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bergmanngabor - We would like to upgrade OpenApi generator to 5.x project-wise in IQS, I think it is reasonable to use newer version, because later we might face issues caused by the outdated generator.
It seems, that out Open API generator causes many issues in the client, which may break this extension. Therefore we do not upgrade the Open API version for now. The old version (4.2.3.) stays instead. This means, that this issue becomes irrelevant. |