Skip to content
This repository has been archived by the owner on Oct 16, 2023. It is now read-only.

Do you use the gafferpy python shell? #885

Closed
n3101 opened this issue Apr 28, 2021 · 8 comments
Closed

Do you use the gafferpy python shell? #885

n3101 opened this issue Apr 28, 2021 · 8 comments
Labels

Comments

@n3101
Copy link

n3101 commented Apr 28, 2021

We now have two gaffer python shells - the older gafferpy and the newer, partly developed, fishbowl dynamic shell. While one is more established it has some issues and detractors. The newer shell needs work, but possibly has more potential in the long run.

What would your reaction be if our strategy was to develop fishbowl and then retire the older shell?

What things must we add to fishbowl to make it more attractive than gafferpy?

@n3101 n3101 added the question label Apr 28, 2021
@sw96411
Copy link

sw96411 commented Apr 29, 2021

I'm quite a big user of the existing shell. In principal, I'm quite a fan of the idea of fishbowl. Would be interesting to think about migration path, and what exactly "retiring" the old shell means. As the old shell is essentially a wrapper for the REST API, I'm hopeful that this plan wouldn't impose an immediate cost on folks who are using the existing library.

Would be great to hear more, and have a chance to lobby for my pet peeves being addressed in the move :-)

@t92549
Copy link
Contributor

t92549 commented Apr 29, 2021

@sw96411 I think a discussion with some key users would be great. It would be useful to hear what design features can be addressed and added while developing fishbowl, as well as adding the missing features.
I also think it is safe to say that when fishbowl becomes feature complete we wouldn't just drop gafferpy immediately but it would not receive new features as quickly for example.

@n3101
Copy link
Author

n3101 commented May 4, 2021

@sw96411 Bring on your pet peeves

@sw96411
Copy link

sw96411 commented May 5, 2021

:-) Just a few things really.

Firstly, an observation on fishbowl that you are probably going to change anyway - at the moment IIRC it just dumps an API binding to the folder 'generated' under whatever the current working directory of the python shell is. This is a risky move for a couple of reasons (I might not have write access to that directory, I might be trying to bind to two different Gaffer instances, I might be sharing a python installation with different users, I might already have a folder called "generated" at that place in my filesystem...).

I'd suggest at least allowing the destination to be specified and defaulting to a tempfile. You may also be able to use importlib tricks and filelike objects to automatically import the relevant libraries, and maybe even do so without needing to write a file to the actual filesystem.

Re: GafferPy itself, most of the usability issues I see not related to the actual API itself (which GafferPy can't help, of course!) are about handling results. Navigating results in GafferPy involves dealing with a mix of objects, nested dictionaries (often of size one and keyed by Java FQDNs) and switch statements to deal with directed edges. If you're quite familiar with how Gaffer works, it all makes sense, but it's hard for new users, particularly non-programmers working in Jupyter, to get their heads around.

I'd suggest what's needed is both access to a raw view of the results JSON (as a JSON object, nested-dicts etc) and the ability to tell the API which parts of the results I'm interested in, and then have it create the objects I need as a generator. So I could do something not entirely unlike:

results = gc.execute(my_operation_chain)

for result in results.generate(['source.type AS type', 'source.value AS VALUE', 'my_special_property', ('my_other_property', str.upper)]):
  print(result.type, result.value, result.my_special_property, result.my_other_property)

The idea behind the tuple on my_other_property is I'm telling the API to apply the function str.upper to the property. You could use this to decode a HLLP, list of timestamps etc.

There's different ways to skin this particular cat though, and I can see from fishbowl you've already had some exciting thoughts on that topic!

@t92549
Copy link
Contributor

t92549 commented May 5, 2021

@sw96411 I think these are both good ideas.
Being able to load the library straight in from memory, and optionally save it to disk, seems a lot cleaner.
Also I like your idea for making the outputs easier to handle and reason with. I think a raw json response should be still available but some form of response which is easier to query and process would make the API a lot easier and nicer to use.

@n3101 n3101 added this to the v2.0.0 milestone Aug 24, 2021
@t92549
Copy link
Contributor

t92549 commented Nov 24, 2021

The decision has been made that gafferpy will be deprecated in the last few Gaffer 1 releases. With Gaffer 2 it will be removed and replaced by a more functional fishbowl. The deprecation will act as a way to show users that gafferpy is for Gaffer 1 graphs, fishbowl is for Gaffer 2 graphs.
@sw96411 how does this sound to you?

@t92549 t92549 modified the milestones: v2.0.0, v1.21.0 Nov 24, 2021
@t92549
Copy link
Contributor

t92549 commented Nov 24, 2021

See #951

@t92549 t92549 closed this as completed Nov 24, 2021
@t92549 t92549 removed this from the v1.21.0 milestone Nov 24, 2021
@t92549
Copy link
Contributor

t92549 commented Jul 28, 2022

The decision has been made that gafferpy will be deprecated in the last few Gaffer 1 releases. With Gaffer 2 it will be removed and replaced by a more functional fishbowl. The deprecation will act as a way to show users that gafferpy is for Gaffer 1 graphs, fishbowl is for Gaffer 2 graphs. @sw96411 how does this sound to you?

Coming back to say that this approach was incorrect and we have changed it. The intended goal was to make use of fishbowl more as it reduces maintenance. However, deleting the existing gafferpy api was not a great way to go about it. Instead, we have integrated the two to make use of fishbowl's advantages while keeping the existing gafferpy: #981.

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

No branches or pull requests

3 participants