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.

Sign up for GitHub

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

Jump to bottom

Fix parameterSensitivities by removing unsupported enum #353

Closed
wants to merge 1 commit into from
Closed

Fix parameterSensitivities by removing unsupported enum #353

wants to merge 1 commit into from

Conversation

michaelarnauts
Copy link
Contributor

@michaelarnauts michaelarnauts commented Jun 14, 2024 •
edited
Loading

This PR fixes creation of ParameterContexts. There are a few issues with the current implementation:

Also, I noticed the following issues when trying to fix this:

  • generate_api_client.sh uses a invalid domain to fetch swagger-codegen-cli, it's also using an old version.
  • The self.logger["package_logger"] = logging.getLogger("swagger_client") line was incorrectly updated in the past PR. Running the codegen sets it to logging.getLogger("nifi") as it was before the last change.

I'll move these changes to a different PR.

Fixes #325

@michaelarnauts michaelarnauts force-pushed the fix-parameter-sensitivities branch 3 times, most recently from 8b01275 to 72d29b4 Compare June 19, 2024 09:23
@michaelarnauts michaelarnauts mentioned this pull request Jun 19, 2024
Merged
@michaelarnauts
Copy link
Contributor Author

Another fix could be to modify the swagger_templates/model.mustache file. It seems that this doesn't impact other usages.

Chaffelson
Chaffelson reviewed Jun 19, 2024
View reviewed changes
Copy link
Owner

@Chaffelson Chaffelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another one that should be fixed in Apache NiFi, but we can patch locally.
I try not to commit local fixes until I know the next version of NiFi won't perpetuate the issue, so as with your other PR let me know if you'd like me to handle it or if you would like the contributor credit as you found and fixed it.

@michaelarnauts
Copy link
Contributor Author

This is another one that should be fixed in Apache NiFi, but we can patch locally. I try not to commit local fixes until I know the next version of NiFi won't perpetuate the issue, so as with your other PR let me know if you'd like me to handle it or if you would like the contributor credit as you found and fixed it.

I'm okay with trying to upstream the changes to NiFi itself, but I can't find the swagger definition in their repository. Can you point me to the right direction?

@michaelarnauts
Copy link
Contributor Author

This is another one that should be fixed in Apache NiFi, but we can patch locally. I try not to commit local fixes until I know the next version of NiFi won't perpetuate the issue, so as with your other PR let me know if you'd like me to handle it or if you would like the contributor credit as you found and fixed it.

I'm okay with trying to upstream the changes to NiFi itself, but I can't find the swagger definition in their repository. Can you point me to the right direction?

Hmm, I've found some annotations here (https://github.com/apache/nifi/blob/main/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java#L282), I assume the swagger file is generated from those annotations?

I'm not sure where the fix should occur however, since from the NiFi standpoint, it's not wrong to say they respond with a string.

@Chaffelson
Copy link
Owner

This is another one that should be fixed in Apache NiFi, but we can patch locally. I try not to commit local fixes until I know the next version of NiFi won't perpetuate the issue, so as with your other PR let me know if you'd like me to handle it or if you would like the contributor credit as you found and fixed it.

I'm okay with trying to upstream the changes to NiFi itself, but I can't find the swagger definition in their repository. Can you point me to the right direction?

Hmm, I've found some annotations here (https://github.com/apache/nifi/blob/main/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java#L282), I assume the swagger file is generated from those annotations?

I'm not sure where the fix should occur however, since from the NiFi standpoint, it's not wrong to say they respond with a string.

Generally we would discuss it in the NiFi community - either on the Developer mailing list or in the community Slack.

You may wish to join the Developer mailing list, and then send an email with the couple of fixes wanted here to generate the discussion on what would be best. Alternatively, if you'd like to join the Slack community and join the nifi-http-clients channel we can have the conversation with the other maintainers there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Chaffelson Chaffelson Chaffelson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error "Invalid keys in parameter_sensitivities" after calling get_parameter_providers
2 participants
@michaelarnauts @Chaffelson