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

Issue/1192 garden connection buttons #4

Draft
wants to merge 7 commits into
base: garden_connection_config_feature
Choose a base branch
from

Conversation

jlrpnbbngtn
Copy link
Owner

@jlrpnbbngtn jlrpnbbngtn commented Jan 18, 2022

Overview

There are five commits in this PR to be concerned with:

  1. Typo: A single misspelt word in a Python string. Safe to ignore.
  2. Massive eslint refactor: These changes are mostly automatically fixed via eslint. Care was taken to ensure the correct version of eslint was used with the correct .eslintrc.json file. How we came to the place where there were thousands of errors is unknown, but the easy fixes are in place with this commit. There are further corrections that need to be made by hand and will occur in a later commit. Should be safe to trust.
  3. Alter UI to work with server data validation: recommend testing in conjunction with commit 4
    a. Testing revealed that GardenBaseSchema was returning error messages that were not useful in their current state (strings with single quotes embedded in them don't play well with JSON).
    b. Code was added to disallow updating a garden to have no connection parameters at all.
    c. Code was added to interpret error messages received because of missing non-optional parameters and update the UI with that information.
  4. Add import/export buttons & functionality: as the name says
  5. Fixed remaining eslint errors. Should be safe to trust.

Testing

  1. We assume testing commences from commit 4 or 5. Using the usual methods, start parent and child instances of beer garden.
  2. Access the garden view page for the child garden.
  3. Set the connection type to HTTP and update the Host Name and Port fields to their correct values (e.g., bg-child1 and 2447).
  4. Select the Export Connection button. Examine the JSON file saved to the local file system to verify it is a faithful representation of the updates to the form.
  5. Delete all fields for HTTP (Host Name, Port and URL Prefix). Press the Save Configuration button. Note that the UI updates with an appropriate error message about not allowing both HTTP and Stomp configuration to be empty.
  6. Select the Import Connection button and chose the JSON file that was just saved. Note that the UI updates with the previous values for the connection.
  7. Again select the "Save Configuration" button. Note that the previous error message is cleared. Use the "Sync" button to verify that the garden configuration has been updated on the server (or examine the database directly).
  8. Set the Port to a disallowed value (e.g. zero or a negative number). Again select the "Save Configuration" button. Note that the UI updates to show that there is an error in the HTTP configuration (this would be useful if you had selected the STOMP tab before pressing the button). Note that on the HTTP tab there is a validation error associated with the port field and that it communicates the error actually received from the server (Value out of range for ports in this case).
  9. Delete the Port field completely and choose the "Save Configuration" button again. Note that the invalidation message associated with the port field updates.
  10. Reset the Port field to a legal value (e.g., 2447). Set a single optional value on the Stomp tab. For example, set the Send Destination to a non-empty string. Choose the "Save Configuration" button again. Note that UI updates to alert of the stomp connection errors and the problematic fields are highlighted on the STOMP tab.
  11. Delete the previously set stomp field so that every field on the stomp tab is blank. Select the Connection Type as STOMP and press the "Save Configuration" button again. Verify that the UI updates with a message indicating an empty connection is not allowed.
  12. Optional: Alter the saved JSON file and reload it using the button. Verify that only the UI form is updated. In other words, there is no attempt to update the server until the "Save Configuration" button is pressed. This behavior is different from the Import Jobs functionality.

Other

  1. Leaving the host name blank on the http tab doesn't always produce an error. In some cases it causes the server to throw an exception (AttributeError: 'dict' object has no attribute 'id' from a malformed call to a logger) and in other cases it updates without issue with the disallowed blank field. Other times everything works as expected. This behavior is believed to not be related to this work and will be investigated under a different ticket.

@@ -19,17 +19,13 @@ class GardenBaseSchema(Schema):
def validate_all_keys(self, post_load_data, original_data, **kwargs):
# do not allow extraneous keys when operating on a dictionary
if isinstance(original_data, dict):
extra_args = original_data.keys() - post_load_data.keys()

extra_args = post_load_data.keys() - original_data.keys()
Copy link
Owner Author

Choose a reason for hiding this comment

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

The changes made here in this commit are not actually related to any work in adding the buttons. But because this line was backwards, this code was firing and it revealed that the error messages were uninterpretable.

@@ -307,6 +387,12 @@ export default function gardenService($http) {
'stomp.username',
'stomp.password',
'stomp.ssl',
{
Copy link
Owner Author

Choose a reason for hiding this comment

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

angular-schema-forms is abandonware. It hasn't been updated in 5 years and it has outstanding PRs from 7 years ago. It has problems. At any rate, many cycles were wasted trying to get the form to properly update on the page when the data was updated, all for nothing. The issue appears to be with the library itself. This message to the user to refresh the page is acknowledgement of defeat. :(

@@ -9,7 +9,7 @@
"es6": true
},
"parserOptions": {
"ecmaVersion": 6,
"ecmaVersion": 2018,
Copy link
Owner Author

@jlrpnbbngtn jlrpnbbngtn Jan 18, 2022

Choose a reason for hiding this comment

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

It doesn't really matter what this is set to, as long as it's set to something sensible. ecmaVersion 2018 is very nearly ecmaVersion 6. It just adds a few constructs that are more modern. In any case, babel compiles to the same version of JavaScript regardless of this setting. This is only to tell eslint which version to try to parse.

@jlrpnbbngtn jlrpnbbngtn force-pushed the issue/1192-garden-connection-buttons branch from e014106 to 187d0e7 Compare January 18, 2022 14:08
@jlrpnbbngtn jlrpnbbngtn marked this pull request as ready for review January 18, 2022 14:19
@jlrpnbbngtn jlrpnbbngtn force-pushed the garden_connection_config_feature branch from 97c5701 to f76c1d7 Compare January 25, 2022 17:52
@jlrpnbbngtn jlrpnbbngtn marked this pull request as draft February 2, 2022 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant