Remove the need for tracking maap.cfg file location

[grallewellyn]

Right now, our environment.yml needs to set the environment variable MAAP_CONF: '/maap-py/' to be able to instantiate MAAP() in a notebook in the ADE

[chuckwondo]

@sujen1412, @grallewellyn mentioned in #801 that you wanted to work on this. I already have a working solution, if you're interested. As I mentioned in #801, it's a matter of making maap.cfg a package resource.

[sujen1412]

We can add the maap.cfg as a package resource to provide valid defaults.

Although thinking further ahead we were wondering if we need a maap.cfg ? We were thinking that other than providing defaults, the config values in maap.cfg should ideally be advertised by the api its pointing to.
For example the user could override the maap_host when instantiating the maap object and the other values are populated subsequently by calling the api endpoint GET /config

[sujen1412]

So we can proceed with #801 by packaging maap.cfg as a resource and in this ticket I can address the part where the api can advertise the config to its clients.

[chuckwondo]

Sounds good to me. I'll help with #801.

[chuckwondo]

@grallewellyn, I believe my work on MAAP-Project/maap-py#78 addressed this issue. Does that suffice to close this issue as complete?

[grallewellyn]

Yes, closed

[grallewellyn]

In the ADE trying to run

from maap.maap import MAAP
maap = MAAP(maap_host='api.dit.maap-project.org')

we are getting

OSError                                   Traceback (most recent call last)
Cell In[3], line 2
      1 from maap.maap import MAAP
----> 2 maap = MAAP(maap_host='api.dit.maap-project.org')

File /maap-py/maap/maap.py:39, in MAAP.__init__(self, maap_host, config_file_path)
     36 self.config = ConfigParser()
     38 # Adding this for newer capability imported from SISTER, leaving the rest of config imports as is
---> 39 self._singlelton_config = ConfigReader(maap_host=maap_host, config_file_path=config_file_path)
     41 config_paths = list(
     42     map(
     43         self._get_config_path,
   (...)
     50     )
     51 )
     53 for loc in config_paths:

File /maap-py/maap/singleton.py:6, in Singleton.__call__(cls, *args, **kwargs)
      4 def __call__(cls, *args, **kwargs):
      5     if cls not in cls._instances:
----> 6         cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
      7     return cls._instances[cls]

File /maap-py/maap/config_reader.py:42, in ConfigReader.__init__(self, maap_host, config_file_path)
     40         pass
     41 if not configfile_present:
---> 42     raise IOError("No maap.cfg file found. Locations checked: " + '; '.join(config_paths))
     44 if maap_host is None:
     45     if not self.__config.has_option('service', 'maap_host'):

OSError: No maap.cfg file found. Locations checked: maap.cfg; ./maap.cfg; /projects/maap.cfg; /maap-py/maap.cfg

I am running an image right now attempting to fix this by commenting out: https://github.com/MAAP-Project/maap-py/blob/develop/maap/config_reader.py#L41
We are confused why we were able to successfully test this before without error

cc @chuckwondo

[grallewellyn]

Removing this line: https://github.com/MAAP-Project/maap-py/blob/develop/maap/config_reader.py#L41 worked

@chuckwondo What are your thoughts on this change?

[chuckwondo]

@grallewellyn, offhand, I'd say I need a bit more information, but that perhaps the env var MAAP_CONF is set, and thus the bundled maap.cfg file is not being read. Is MAAP_CONF set by default in the ADE? If so, it should no longer be set. Unset (unset MAAP_CONF) and try again to see if you still get the error. There should be no need to modify the code.

[sujen1412]

MAAP_CONF is currently being set in all envs.
https://github.com/MAAP-Project/maap-workspaces/blob/main/base_images/vanilla/environment.yml#L51

[grallewellyn]

I am building test images right now removing MAAP_CONF from all envs
I was under the impression MAAP_CONF didn't need to be set anymore, not that it shouldn't be set. I will update once I test these images

[grallewellyn]

Removing MAAP_CONF from the envs worked and I can successfully instantiate maap-py and use its functions

[chuckwondo]

Great to hear!

To clarify, it's not that it shouldn't be set, but more specifically, that it shouldn't be set to a place where a maap.cfg file doesn't exist. It can be set, so long as a maap.cfg file can be found there.

By default, it shouldn't be set by the environment because maap.cfg is now bundled with the package, but if a user wants to override what comes bundled, then the user can set MAAP_CONF as they like.

[grallewellyn]

Thank you for clarifying!

[grallewellyn]

Had to undo this change because it was causing a bug for the v3.1.5 release, we will issue with the next release
MAAP-Project/maap-py#89
cc @bsatoriu

[chuckwondo]

@grallewellyn can you clarify? What happened?

[grallewellyn]

This is the ticket what we made to address the bug that was arising from maap-py: #965

The changes Brian proposed and we implemented for v3.1.5:
https://github.com/MAAP-Project/maap-py/tree/bsatoriu-patch-dit

• Moved maap.cfg back to root directory; used the standard config reader since the new version was buggy. Since we're moving this config to the API soon anyway, this solution would suffice for 3.1.5.

https://github.com/MAAP-Project/jupyter-server-extension/tree/bsatoriu-patch-dit

• Added the xmltodict library but no other additional dependencies for now. We'll need to test the full list of dependencies required for jupyter-server-extension but I don't see why that ticket can't be addressed in the next sprint.

This is the maap-py fix we merged to solve the error: MAAP-Project/maap-py#89

[chuckwondo]

But what exactly was the problem and how did it show up? I feel like we are now unfortunately losing the non-trivial benefits that came with properly handling maap.cfg as a resource. I don't think it was unnecessary to undo all of these changes. I wish you would have simply reached out to me to help resolve whatever problem was encountered, as I'm confident that we could have fixed the issue without completely undoing the changes I made here.

[grallewellyn]

Since you commented on the issue I made explaining the bug which specifically highlighted the upgrade of maap-py from v3.1.4 to v3.1.5 being the source of the bug, I thought you were aware of the situation, my apologies I should have reached out to you directly. We also decided that extracting maap.cfg out was a rather large change and it might be better suited for our next platform release which will be 4.0.0.

The problem was manifesting as not being able to view your jobs in the Jobs UI which was a result of not successfully getting the user profile which was a result of maap-py and jupyter server extension not being properly activated. We were unable to find very useful details in the logs, so we just tested images reverting back maap-py to discover that was the issue and then tried to narrow it down from there

[chuckwondo]

Unfortunately, in the issue you linked, it was not at all obvious to me that it had anything to do with the maap.cfg changes.

[grallewellyn]

If we move the config to the API in this next release (like we are planning to do), this issue would no longer be relevant