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.

Sign up for GitHub

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

Jump to bottom

Fix indexes #849

Merged
merged 10 commits into from
May 27, 2022
Merged

Fix indexes #849

merged 10 commits into from
May 27, 2022

Conversation

aGuttman
Copy link
Contributor

Change HASHED to ASCENDING and removed sparse from GEOSPHERE for documentdb compatibility

@aGuttman
Fix indexes
c65a4b2 
Change HASHED to ASCENDING and removed sparse from GEOSPHERE for documentdb compatibility
@aGuttman
Copy link
Contributor Author

Fixes one part of e-mission/e-mission-docs#721

@aGuttman aGuttman mentioned this pull request May 10, 2022
Closed
aGuttman and others added 5 commits May 16, 2022 14:11
@aGuttman
Set DB Name in URL
12b5210 
Allows DB name to be set in the URL of a config file. Defaults to "Stage_database" is no name is provided.
@aGuttman
Hack to get fix moveToLongTerm Bug
6a84b42 
Hack to avoid dividing non-millisecond timestamps by 1000. Gets tests to pass.
@shankari
Remove the code path for the "old style" format
5e53ee0 
We added the "new style" location format in 2015.
We retained the code to process the "old style" format for backwards
compatibility, since the clients would not necessarily be upgraded
immediately.

However:
- all clients should have been updated by now
- we have unpublished that client
- we are creating a new client to talk to the new server

Let's just remove the old style code path since we don't need it any
more, and there's no point in keeping bitrotted code.

This fixes e-mission/e-mission-docs#721 (comment)

Changes:
- change the data from the old format to the new format
- ensure that we load the new format
- remove `format_location_raw` and always return `format_location_simple`

Testing done:
- `emission/tests//netTests/TestBuiltinUserCacheHandlerInput.py` and
- `emission/tests//netTests/TestBuiltinUserCacheHandlerOutput.py` both pass
@shankari
Change the formatter tests to also test the "new style" format
65a32fc 
Instead of the old style format

This fixes the one test failure in
#851
@aGuttman
Merge branch 'db_compatibility' into remove_obsolete_format_simple
36df95a
@shankari
Copy link
Contributor

@aGuttman any ETA for this? I would like to merge #853
to finish up the server changes, and don't want to cause merge conflicts with your code.

@aGuttman
Copy link
Contributor Author

aGuttman commented May 25, 2022 •
edited
Loading

The name configuration for TestMongoAuth.py is easy enough, I can finalize those changes today, but I'm first having trouble getting the test to pass without any changes. I get auth errors when it tries to create the first admin user, which as I understand, shouldn't be happening: https://www.mongodb.com/docs/manual/core/localhost-exception/#std-label-localhost-exception

The test itself seems to be written in a way were it expects this to work. I'm doing this on a fresh db without anything in it, so that shouldn't be the problem. And once this works, I can get the db name to be dynamic, but right now the only way I've been able to make it work is by manually adding a admin user via the mongo shell, connecting with that users credentials and then creating/using the new admin user the test creates. And this only works if you only run one test at a time, adding the admin user manually each time. If this is the intended use (I don't think it is), this test seems a little pointless to configure nicely, since there is so much manual work anyway.

@shankari
Copy link
Contributor

@aGuttman have you started mongodb with the --auth command?

# Implemented outside tests because it requires mongod to be started with the
# --auth opton to pass
# $ mongod --auth
# if mongodb is run without `--auth`, the readonly test is guaranteed to fail

@aGuttman
Copy link
Contributor Author

Yes. Without the auth option the users can be created but the assertRaises() fail in the tests since there is no access restriction. With auth I can only seem to create the first user in the mongo shell.

@shankari
Copy link
Contributor

shankari commented May 25, 2022 •
edited
Loading

Hm, interesting. Please file an issue for investigating that as well.

So the next tasks are:

  • file an issue for the admin user creation issue
    • add a test to TestMongodbAuth to test with a different DB as well (e.g. testReadWriteUserCustomDB)
      • no additional config, just copy-paste the previous test and make your changes using the clunky approach above
    • change setup/db_auth.py to support a custom database as well (just add it under "Variables to change")
  • file an issue for federating data
    • remove the _reset_url code from the timeseries

@aGuttman I think that's it!

This was referenced May 25, 2022
Open
Open
aGuttman added 4 commits May 25, 2022 20:14
@aGuttman
Update TestMongodbAuth.py
2cffd3b 
Got the tests to pass (so long as they are run one at a time, with correct admin already entered). Added functions to test creating new users with correct restrictions with a custom db name.
@aGuttman
Update abstract_timeseries.py
4cf770e 
remove hacky dead code for connecting to multiple databases
@aGuttman
Update db_auth.py
5a15460 
Set database name as hardcoded global var instead of hardcoded in several, non-aggregated places.
@aGuttman
Update TestMongodbAuth.py
947c7b8 
Uncommenting a block of code that was left commented out on accident
@shankari shankari merged commit 2909ef5 into e-mission:master May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shankari shankari shankari approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aGuttman @shankari