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

{Core} Fix #24542: Support JMESPath Community dependencies and assets. #25590

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

springcomp
Copy link

@springcomp springcomp commented Feb 27, 2023

Related Command

As this PR addresses the JMESPath language used in the --query command-line argument, it applies to all az CLI commands 😱.

Description

This PR addresses #24542 by updating the current dependency on the JMESPath language used in the --query command-line argument.

Important: as AZ CLI has a dependency over knack, the following pull request must be reviewed and accepted first before merging this one. For this reason, most of the checks and unit-tests performed as part of this PR are failing.

Testing Guide

az CLI uses the JMESPath language to power its --query command-line argument.
For instance, consider the following current behaviour:

  • az storage account list --query "[].{location: location, name:name}":
[
  { "location": "westeurope", "name": "storageaccount-001" },
  { "location": "westeurope", "name": "storageaccount-002" },
  { "location": "northeurope", "name": "storageaccount-003" },
  { "location": "westus", "name": "storageaccount-004" },
  { "location": "westus", "name": "storageaccount-005" }
]

Here is a new query that can be performed using JMESPath Community:

az storage account list \
  --query "group_by([].{loc: location, n:name}, &loc).*.let( {key: [0].loc, values: [*].n}, &{key: key, values: values})|from_items(zip([*].key, [*].values))"

with the corresponding result:

{
  "westeurope": [ "storageaccount-001", "storageaccount-002" ],
  "northeurope": [ "storageaccount-003" ],
  "westus": [ "storageaccount-004", "storageaccount-005" ]
}

image

While there is definitely a learning curve to JMESPath, this example features most of the improvements we’ve made to the specification.

The full expression can be broken down into chunks:

  • group_by([].{loc: location, n: name}, &loc): creates a new hash with the subset of interesting location and name properties from the original object. It uses the new group_by() function to split the result by location.
  • .*: is a value projection to create an array that can be iterated upon subsequently.
  • let( {key: [0].loc, values: [*].n}, &{key:key, values:values}): expression that is evaluated for each item in the projected array. It uses the new let() function to take advantage of lexical scoping.
    • Its first argument initializes a new scope with the two key and values properties that contain the value of the loc property from the first array entry – as all entries contain the same location at this stage – and the list of names respectively.
    • Its second argument creates a hash with the two aforementioned properties.
  • |: the pipe stops the projection so that later expressions operate on the full array instead of being evaluated iteratively on each array element.
  • from_items(zip([*].key, [*].values)): creates the expected result.
    • It uses the new zip() function to combine the list of key elements and the values arrays into a single entity.
    • It then uses the new from_items() functions to combine those two lists – like for like – to produce the resulting object shown above.

Edit: in fact, this expression can be simplified further as we do not really need to use the let() function. The following simpler expression will work just as well:
"from_items(group_by([].{loc:location, n:name}, &loc).*.{key:[0].loc, values:[*].n}.zip([*].key, [*].values))"
Definitely simpler 😁

Overview

This PR is a draft as I would like to make the most effort to help include an updated JMESPath language with an improved feature set to az CLI. However, I’m not sure what would be the reasonable changes to do or if it should be broken into multiple pull requests. I would also likely need guidance over how to best fill the PR description template.

This PR makes the following changes:

  • Replaces all mentions of the jmespath.org original web site to the new jmespath.site.
  • Replaces original dependencies over jmespath Python package to the new jmespath-community Python package.
  • Replaces original dependencies over jmespath-terminal (jpterm helper) Python CLI tool to the new jmespath-community-terminal tool in Docker images.
  • Replaces original github.com/jmespath/jp CLI tool by the new github.com/jmespath-community/jp CLI tool in Docker images.
  • Updates minimum Python version to 3.7.16 for some Docker image.
  • Updates various documentation pages while keeping the same LICENSE requirements.

I’m not sure if all the documentation pages must be updated though.

JMESPath Community

This PR aims to include JMESPath Community assets into az CLI.

JMESPath Community is a new initiative to bring improvements to and steward the specification going forward,
as the language and its tools no longer appear to be maintained. As its first milestone,
JMESPath Community includes the following improvements to the original specification:

  • JEP-11 Lexical Scoping
  • JEP-12a Raw String Literals
  • JEP-13 Object Manipulation functions
  • JEP-14 String functions
  • JEP-15 String Slices
  • JEP-16 Arithmetic Expressions
  • JEP-17 Root Reference
  • JEP-18 Grouping
  • JEP-19 Standardized Evaluation of Pipe Expressions

JMESPath Community also aims to be an umbrella for specific programming language implementations. It currently hosts the following 100% compliant implementations:

I’m also the co-author of the JMESPath.Net implementation 🤐.

For the purpose of az CLI, JMESPath Community can be configured to maintain backwards compatibility with the original jmespath Python package. This PR adds the necessary enable_legacy_literals=True flag in various places to prevent breaking existing syntax and scripts. Incidentally, I think the usage of JMESPath’s Options parameter is a bit too scattered in various places and could gain from being more centralized. I will think of a PR for that in the future.

The various built-in Docker images re-introduce jpterm which has seen its dependencies updated and no longer breaks the CI.

The accompanying jp executable has also been brought up to standards and is included in this PR.


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link

Validation for Azure CLI Full Test Starting...

Thanks for your contribution!

@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Feb 27, 2023
@ghost
Copy link

ghost commented Feb 27, 2023

Thank you for your contribution springcomp! We will review the pull request and get back to you soon.

@yonzhan
Copy link
Collaborator

yonzhan commented Feb 27, 2023

JMESPath Community

@springcomp springcomp changed the title Supports JMESPath Community dependencies and assets. {Core} Supports JMESPath Community dependencies and assets. Feb 27, 2023
@springcomp springcomp changed the title {Core} Supports JMESPath Community dependencies and assets. {Core} Fix #24542: Support JMESPath Community dependencies and assets. Feb 27, 2023
@jiasli
Copy link
Member

jiasli commented Aug 21, 2023

Thank you very much @springcomp for the notification and great work.

Because of the ever-increasing usage of Azure CLI, we are focusing more and more on maintaining a stable and consistent user experience. https://jmespath.site/main/#libraries mentions the Python version of jmespath-community is not Fully compliant with the original version of JMESPath. Even though enable_legacy_literals=True is set, we still need extra effort to fully test jmespath-community to make sure there are no unexpected breaking changes.

However, due to other high priority tasks, this has not been planned yet. We will discuss this PR with our Product Managers and see how we should move on.

For now, instructing users to use jp instead of Azure CLI's built in --query argument sounds like a more plausible solution, as this will defer the decision of using which version of JMESPath to the user.

@springcomp
Copy link
Author

@jiasli Thank you for your honest feedback.

You are right, JMESPath Community is not fully compliant.

I have made efforts to make sure compatibility is preserved as much as is possible. The unit-tests in AZ CLI are all working fine with the compatibility switch. However, there are still a couple of opinionated differences with the original implementation 😏. For the sake of transparency, these are tracked here:

Instructing users to us jp might be a good interim solution.

Thank you for your time.

@@ -2262,7 +2262,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI

---------------------------------------------------------

jmespath 0.10.0 - MIT
jmespath-community 1.1.0 - MIT
Copy link
Member

Choose a reason for hiding this comment

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

Does the copyright of jmespath-community still belong to Amazon?

Copy link
Author

Choose a reason for hiding this comment

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

We have forked the repository, so I’m not sure what is the best practice.
Also, I’m not familiar at all with respect to licenses and copyright 😱.

What do you think in your experience ?

Dockerfile Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants