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

Support for the new 'createmeta' API in Jira 8.4+ #1527

Merged
merged 6 commits into from
Jan 29, 2023

Conversation

pmilosev
Copy link
Contributor

@pmilosev pmilosev commented Nov 8, 2022

Fix for #1462

This is an alternative to #1503, introducing new API functions and not trying to reconstruct the old API. Even when filtering is used in the old API to get metadata for single project and issue type, the results are still not identical (differ in structure).

I'm not sure if any tests can be written testing against different versions of JIRA. At the moment I have tested manually against JIRA 7.13.8, 8.22.6 and 9.3.1.

@pmilosev
Copy link
Contributor Author

pmilosev commented Nov 8, 2022

Am I supposed to add a label to the PR or the admins ?
I see a failing check for it.

@dshvedchenko
Copy link

you are right, results a different. i will work on make them similar.
but in your code you also need to pay attention that _get_json is not paging aware, and will return only first part of metadata.

this is why I'm using _fetch_pages , coz it gets all metadata

@pmilosev
Copy link
Contributor Author

pmilosev commented Nov 9, 2022

Thanks for reminding me about the paging ... I remember writing a TODO and later not looking into it.
This should not be a problem for the new API though as it only returns results (the fields) for one issue type, so only if there are over 50 fields in a issue type the paging will be an issue.

I still want to discourage you from trying to make the old API behave like the new one:

  • you need complicated code that iterates through all projects (all filtered projects) and all (filtered) issue types, composing the result in the same structure as earlier; but then you are going against the JIRA API decisions
  • if you don't do that and only reconstruct the results from the first project and first issue type (I remeber seeing that in the PR) then you are implicitely introducing filters that the callers are not expecting; so not really a benefit, but a source for potential bugs
  • currently Jira 9 is not supported (already breaks), so there are no depndant projects that work and will break if we deprecate the old API and provide a new one; Since Atlassian decided to change the architecture of their APIs I think the best is to explicitely follow those decisions

Anyway these are conceptual decisions that need to be made by the maintainers.

@pmilosev
Copy link
Contributor Author

pmilosev commented Nov 9, 2022

It appears the original createmeta function is also using _get_json so I will not change this unless it causes an issue:

  • The new API is filtered by design so if paging was not an issue before it will not be now
  • This whole technology stack is new for me so it's more likely I will introduce bugs
  • I have limited setup / capacity for testing, which combined with previous point makes me want to introduce as few changes as possible

pmilosev added a commit to netceteragroup/django-DefectDojo that referenced this pull request Nov 10, 2022

Verified

This commit was signed with the committer’s verified signature.
aduh95 Antoine du Hamel
Fixes: DefectDojo#6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).
pna-nca pushed a commit to netceteragroup/django-DefectDojo that referenced this pull request Nov 10, 2022

Verified

This commit was signed with the committer’s verified signature.
aduh95 Antoine du Hamel
Fixes: DefectDojo#6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).
@studioj studioj added the bug label Nov 12, 2022
Copy link
Collaborator

@studioj studioj left a comment

Choose a reason for hiding this comment

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

it would be nice if you would help the user more in the exception message

pna-nca pushed a commit to netceteragroup/django-DefectDojo that referenced this pull request Nov 14, 2022
Fixes: DefectDojo#6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).
@adehad adehad added feature and removed bug labels Nov 14, 2022
@adehad
Copy link
Contributor

adehad commented Nov 14, 2022

I'm not sure if any tests can be written testing against different versions of JIRA. At the moment I have tested manually against JIRA 7.13.8, 8.22.6 and 9.3.1.

The docker image allows specifying a version so we should be able to configure a test case to run on the higher version, example:

run: docker run -dit -p 2990:2990 --name jira addono/jira-software-standalone --version ${{ matrix.jira-version }}

jira/client.py Outdated
"This API have been deprecated in JIRA 8.4 and is removed in JIRA 9.0."
"Use 'createmeta_issuetypes' and 'createmeta_fieldtypes' instead.",
DeprecationWarning,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

we probably want to change the stack level here. Unfortunately the python default isn't super helpful.
We can check this is a mocked test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't have a lot of experience with Python, so I don't understand what does stack level in this context mean.
Should I do something like:

 warn(
            "This function will fail from Jira 9+. "
            "Use issue_createmeta_issuetypes or issue_createmeta_fieldtypes instead.",
            DeprecationWarning,
            stacklevel=2,
        )

(taken from https://github.com/atlassian-api/atlassian-python-api/pull/1031/files)

Copy link
Contributor

Choose a reason for hiding this comment

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

yep exactly right, here's a useful video explaining this situation: https://www.youtube.com/watch?v=CtFdXBEwYfk

pmilosev added a commit to netceteragroup/django-DefectDojo that referenced this pull request Nov 15, 2022
Fixes: DefectDojo#6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).
@pmilosev
Copy link
Contributor Author

I just discovered that cloud JIRA API is independent from Jira DC/Server API and any clients using it must account for the potential differences: https://community.atlassian.com/t5/Jira-questions/How-can-I-tell-actual-Jira-version-of-a-cloud-instance-for/qaq-p/1483730

According to the cloud Jira API documentation the old createmeta endpoint is still used, so I'll have to account for this in my PR.

@adehad
Copy link
Contributor

adehad commented Nov 16, 2022

I just discovered that cloud JIRA API is independent from Jira DC/Server API and any clients using it must account for the potential differences: https://community.atlassian.com/t5/Jira-questions/How-can-I-tell-actual-Jira-version-of-a-cloud-instance-for/qaq-p/1483730

According to the cloud Jira API documentation the old createmeta endpoint is still used, so I'll have to account for this in my PR.

@pmilosev yes it can get quite tricky.
We have JIRA._is_cloud that can be used to check whether we are on server or cloud too.

We've compiled a list of the API docs here in case you haven't found it already:
https://github.com/pycontribs/jira#jira-rest-api-reference-links

@pmilosev
Copy link
Contributor Author

I've updated the code according to the review comments above and accounted for the difference in Jira Cloud API.
Manual re-test against Jira DC 7.13.8, 8.22.6 and 9.3.1, as well as Jira Cloud 1001 (build number 100210).

pmilosev added a commit to netceteragroup/django-DefectDojo that referenced this pull request Nov 18, 2022
Fixes: DefectDojo#6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).
mtesauro pushed a commit to DefectDojo/django-DefectDojo that referenced this pull request Nov 20, 2022
* Update gcr.io/cloudsql-docker/gce-proxy Docker tag from 1.33.0 to v1.33.1 (helm/defectdojo/values.yaml)

* Refactoring on how jira issue fields are prepared.

No change in behaviour (except perhaps in logging).

* Added support for the new Jira 9+ createmeta API.

Fixes: #6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@pmilosev
Copy link
Contributor Author

@adehad is there something else blocking this PR from being merged ?
I would appreciate if we can get this done while I still have some of my attention to it.

@adehad
Copy link
Contributor

adehad commented Nov 29, 2022

Thanks for the reminder @pmilosev .

Could I ask you to see if we can get some test cases in?

For testing the versions we don't have access to at the moment I would suggest mocking the value of self._cloud and self._version.

Let me know if you need a hand with this style of test. Off the top of my head I believe you would want to patch the the test method, as that is probably the best way to patch them before the setUp() runs. Or otherwise feel free to use a pytest style test.

@mock.patch(...)
def test_createmeta_test_name():

@Roooodie
Copy link

any update on this? issues still happened if we're using Jira 9+.
sorry if I'm bothering people here, I am just curious about the failed checking changes above.
@studioj @pmilosev

@frost9i
Copy link

frost9i commented Jan 17, 2023

Can checks be forced to restart?
It looks like the issue was fixed, but wasn't merged because automated testing failed.

@studioj
Copy link
Collaborator

studioj commented Jan 17, 2023

Can checks be forced to restart? It looks like the issue was fixed, but wasn't merged because automated testing failed.

the server CI part is a bit flakey and more failing than passing :s
@adehad can you merge? I'm guessing a release is needed too

@pmilosev
Copy link
Contributor Author

I wanted to add some tests but am too busy to do so, sorry. I think tests are indeed a good thing and if anyone is able to write them I would really appreciate it. For me the issue is that I'm not experienced with the stack and writing unit tests would require me to spend a decent amount of time learning.

That being said the failing checks predate my changes.
I have tested the change manually against multiple versions of Jira (see previous comments for details) and the change is already used in another product (see DefectDojo links above).

I hope the PR can be merged as it is.

@adehad adehad merged commit 2e14696 into pycontribs:main Jan 29, 2023
@adehad
Copy link
Contributor

adehad commented Jan 29, 2023

Thank you for your contributions @pmilosev and @dshvedchenko.

I've merged it as is for now and raised a separate issue for adding tests.

Hopefully can get it into a release soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants