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

ATLAS-3219: New REST APIs for serviceType. #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZepHakase22
Copy link
Contributor

This pull request supersedes the pull request ATLAS-3180.

The new basic attribute serviceType, introduced by Atlas Team, has no use if it is not searchable ina any way. This pull request solves this problem by adding the following REST APIs.

GET / typedef / servicetype / {servicetype}
GET / typedefs / headers? Servicetype = {serviceType}
GET / typedefs? Servicetype = {servicetype}
GET / enumdef / servicetype / {servicetype}
GET / entitydef / servicetype / {servicetype}
GET / structdef / servicetype / {servicetype}
GET / relationshipdef / servicetype / {servicetype}
DELETE / typedef / servicetype / {servicetype}

The APIs were tested on ATLAS 2.0 and on the new SNAPSHOT 3.0

@mneethiraj
Copy link
Contributor

@ZepHakase22 - instead of adding new REST endpoints (for /servicetype/{serviceType}), I suggest to recognize "serviceType" parameter in FilterUtil.getPredicateFromSearchFilter(SearchFilter filter) so that all methods that use SearchFilter will apply this filter.

@ZepHakase22
Copy link
Contributor Author

ZepHakase22 commented May 29, 2019

@mneethiraj - It was exactly what I did but it works only for

GET / typedefs? serviceype = {servicetype}

how you can see here in the code. But if you want to maintain compatibility with the other REST APIs, that is how the ones for guid and name are made, you need to work in another way, because these APIs refer to the cache.

Furthermore, DELETE, obviously needs a separate code.

public static Predicate getPredicateFromSearchFilter(SearchFilter searchFilter) {
    List<Predicate> predicates = new ArrayList<>();

    final String       type         = searchFilter.getParam(SearchFilter.PARAM_TYPE);
    final String       name         = searchFilter.getParam(SearchFilter.PARAM_NAME);
    final String       supertype    = searchFilter.getParam(SearchFilter.PARAM_SUPERTYPE);
    final String	   serviceType  = searchFilter.getParam(SearchFilter.PARAM_SERVICETYPE);
    
    final String       notSupertype = searchFilter.getParam(SearchFilter.PARAM_NOT_SUPERTYPE);
    final String	   notServiceType = searchFilter.getParam(SearchFilter.PARAM_NOT_SERVICETYPE);
    final List<String> notNames     = searchFilter.getParams(SearchFilter.PARAM_NOT_NAME);

    // Add filter for the type/category
    if (StringUtils.isNotBlank(type)) {
        predicates.add(getTypePredicate(type));
    }

    // Add filter for the name
    if (StringUtils.isNotBlank(name)) {
        predicates.add(getNamePredicate(name));
    }
    
    //Add filter for the serviceType
    if(StringUtils.isNotBlank(serviceType)) {
    	predicates.add(getServiceTypePredicate(serviceType));
    }

    // Add filter for the supertype
    if (StringUtils.isNotBlank(supertype)) {
        predicates.add(getSuperTypePredicate(supertype));
    }

    // Add filter for the supertype negation
    if (StringUtils.isNotBlank(notSupertype)) {
        predicates.add(new NotPredicate(getSuperTypePredicate(notSupertype)));
    }
    
    // Add filter for the serviceType negation
    // NOTE: Creating code for the exclusion of multiple service types is currently useless. 
    // In fact the getSearchFilter in TypeREST.java uses the HttpServletRequest.getParameter(key) 
    // that if the key takes more values it takes only the first the value. Could be usefull
    // to change the getSearchFilter to use getParameterValues instead of getParameter.
    if (StringUtils.isNotBlank(notServiceType)) {
        predicates.add(new NotPredicate(getServiceTypePredicate(notServiceType)));
    }


    // Add filter for the type negation
    if (CollectionUtils.isNotEmpty(notNames)) {
        for (String notName : notNames) {
            predicates.add(new NotPredicate(getNamePredicate(notName)));
        }
    }

    return PredicateUtils.allPredicate(predicates);
}

@ZepHakase22
Copy link
Contributor Author

ZepHakase22 commented May 29, 2019

@mneethiraj -In my opinion you are right because I can't understand why we have a cache for the name, a cache for the guid and now also a cache for the serviceType. I did this only to mantain the compatibility with the existing code.
We could change all the code to have only a generic cache and filtering this cache to resolve the REST APIs. If you want I could better study the code and propose a global comprehensive review of that code.

@ZepHakase22
Copy link
Contributor Author

@mneethiraj - I looked more closely at the code and we could: 1) limit the rest api for the four types one for the post, one for the put, one for the update and one for the cache and then use the predicates. 2) limit to a single cache instead of having a cache for the guid one for the name and one for the service type. If you agree I could start this task.

Copy link
Contributor

@mneethiraj mneethiraj left a comment

Choose a reason for hiding this comment

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

I would suggest to refrain from updating AtlasTypeRegistry and *TypeDefStore classes to support filtering for a specific service-type. Changes in ServiceFitler.java and FilterUtil.java should be enough to enable filtering for a service-type in /typedefs/headers and /typedefs REST API endpoints. Note that these APIs already support filtering for specific type-category (entity/classification/struct/enum/relationship).

@mneethiraj
Copy link
Contributor

bq. Furthermore, DELETE, obviously needs a separate code.
For DELETE by service-type, I would suggest to have the client to perform the following:

  • retrieve the typedefs, filtered-by service-type
  • call to "DELETE /typedefs" with the retrieved typedefs

@ZepHakase22
Copy link
Contributor Author

@mneethiraj - I didn't understand if you propose to add new REST APIs by type
/enumdef /servicetype/hive for example) but use the /typedefs code like
/typedefs?serviceType=hive&&type=enum or not to add just these REST APIs leaving what the REST API already does with the generic parameters.
In my opinion and experience the types are loaded once, they are not many and therefore they occupy little memory while, typically, they are read many times; the introduction of the serviceType cache greatly facilitates these readings with respect to using the cache of only names and guid in terms of cycles. Using predicates will also be very elegant but there are more an more cycles that must be done to aggregate the set of entities that make up the serviceType.
For which I strongly advocate a map by serviceType in typeRegistry.
Finally, I think we can change the typeRegistry to improve it, if the code works fine.

@ZepHakase22
Copy link
Contributor Author

ZepHakase22 commented May 31, 2019

@mneethiraj - Regarding the DELETE you are proposing the fact of not implementing it for the serviceType because moving the work on the client only works if you use the ATLAS API. If for example I use json scripts and curl I have to do all the work from the application client.

@mneethiraj
Copy link
Contributor

bq. moving the work on the client only works if you use the ATLAS API. If for example I use json scripts and curl I have to do all the work from the application client
I am afraid such approach would end up with large number of shallow REST API implementations in Atlas - to support constraints of various clients. We should assume that clients of ATLAS APIs are capable of reading the responses and where necessary, use the response contents in subsequent API calls.

@mneethiraj
Copy link
Contributor

I didn't understand if you propose to add new REST APIs by type
/enumdef /servicetype/hive for example)
I propose no new APIs. Simply enhance existing APIs at /typedefs and /typedefs/headers to support filtering for a given service-type.

As I suggested earlier, please avoid updating AtlasTypeRegistry and *TypeDefStore for this enhancement. If we notice performance issues, we can consider a subsequent update. Let's keep this simple for now - by only adding FilterUtil.getServiceTypePredicate()

@ZepHakase22
Copy link
Contributor Author

ZepHakase22 commented May 31, 2019

I agree absolutely. I don't want to create a complex Rest system to access however the customer wants. I only think that access by service type is much more useful than access by guid foto example because I have on hands soon the types I need.

@ZepHakase22
Copy link
Contributor Author

@mneethiraj - any way the ATLAS-3180 pull request does exactly what you say.

@mneethiraj
Copy link
Contributor

@ZepHakase22 - yes, you are right. The changes in pull request for ATLAS-3180 (#49) looks good. I will merge that patch shortly. Thanks!

@kell18
Copy link

kell18 commented Oct 22, 2020

Guys, what is serviceType? There is not a single piece of docs about it

@mneethiraj
Copy link
Contributor

Guys, what is serviceType? There is not a single piece of docs about it

serviceType is a grouping of type-defs in Atlas. For example: all type-defs for Hive (hive_db, hive_table, hive_column, hive_process, ..) will have serviceType as 'hive'. This enables Atlas UI to filter type-defs list for specific serviceTypes.

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